Skip to content
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

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

aaxelb
Copy link
Contributor

@aaxelb aaxelb commented May 25, 2017

  • Remove the edu.ageconsearch harvester and transformer
  • Add io.tind harvester we could use for any source using Tind IR (from their press releases, looks like just AgEcon so far)
  • Harvest AgEcon in MODS format

Waiting on a fix on filtering by date range.

Copy link
Contributor

@laurenbarker laurenbarker left a 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 💯

@@ -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']),
Copy link
Contributor

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?

Copy link
Contributor Author

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
Copy link
Contributor

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.

Copy link
Contributor Author

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.

Copy link
Contributor Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants