-
Notifications
You must be signed in to change notification settings - Fork 58
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[SHARE-777][Feature][HOLD] Add tind.io harvester for AgEcon #667
base: develop
Are you sure you want to change the base?
Conversation
0697cc4
to
c607117
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just took a quick look since it's on hold 💯
share/transformers/mods.py
Outdated
@@ -240,7 +240,8 @@ class MODSCreativeWork(Parser): | |||
lambda obj: 'invalid' not in obj, | |||
tools.Concat( | |||
tools.Try(ctx['mods:identifier']), | |||
tools.Try(ctx.header['identifier']) | |||
tools.Try(ctx.header['identifier']), | |||
tools.Try(ctx['mods:location']['mods:url']), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@aaxelb, do you want to pull the location url stuff into another PR since this one is on hold and also resolves https://openscience.atlassian.net/browse/SHARE-861?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense, will do.
@@ -1,138 +0,0 @@ | |||
import re |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we want to remove the old transformer? If we re-transform the old data we will still need this code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, I'd kinda rather it be removed, since we'll never get new data for it, but I didn't think about the old data. I'll put it back in for now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the same spirit, I restored the old source config (disabled and with no harvester) so the system knows what to do with the old data.
edu.ageconsearch
harvester and transformerio.tind
harvester we could use for any source using Tind IR (from their press releases, looks like just AgEcon so far)Waiting on a fix on filtering by date range.