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

Update crosswalk table #251

Closed
wants to merge 3 commits into from

Conversation

tmorrell
Copy link
Contributor

This PR adds bio.tools to the main crosswalk table (It was previously added to the crosswalk folder, but never made it the main table). I modified the split.py script so it assumes you're running it in the scripts directory (like aggregate.py). I also enhanced the Zenodo mapping.

@cboettig
Copy link
Member

LGTM. @mbjones ?

@mbjones mbjones added this to the v3.0 milestone Nov 30, 2020
@mbjones
Copy link
Collaborator

mbjones commented Nov 30, 2020

Overall looks good, but I'm curious why the path had to change on split.py to make it relative (and which hard codes UNIX-style path separators now). Was this run from a different directory before? Will it break any CI pipelines? If that looks good to you @cboettig then I am good with merging it into develop. I added it to the v3.0 release milestone.

@tmorrell
Copy link
Contributor Author

Apologies for the delay in getting back to you! I've updated split.py so it uses the same file definitions as aggregate.py, which should avoid the relative path issues.

@moranegg
Copy link
Contributor

Can you divide into 2 separate PR?
PR 251.1 for the crosswalk additional terms (with the aggregate.py changes)
PR 251.2 for the changes in split.py .

Thanks!

PR 251.1 issue doesn't changes the spec and should be resolved for V2.1. label #crosswalk
PR 251.2 if no more comments are added we will merge this PR at the end of March 2023 (included into v3.0)

@progval
Copy link
Member

progval commented Feb 16, 2023

PR 251.2 if no more comments are added we will merge this PR at the end of March 2023 (included into v3.0)

that's a refactoring, no need to wait for v3.0 to merge it

@progval
Copy link
Member

progval commented Apr 24, 2023

Superseded by #291 and #292 (which don't conflict, as they don't edit the main crosswalk)

@progval progval closed this Apr 24, 2023
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.

None yet

5 participants