-
Notifications
You must be signed in to change notification settings - Fork 12
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
niiri prefixes > urn / id are now uuids / project info removed #83
Conversation
Thank you! I have updated the title of this PR to be more explicit and updated sllightly the text in the first comment to add links to each issue explicitly. Can you try and solve the conflicts? (Note: most of the conflicts are in |
Thanks for this! I'll review #82 first and then will come back to this one. But before I forget: could you create an issue about your last comment:
We should be able to correct for that by updating the context but let's do it in another iteration/PR. |
The merge is now ok. I also put back the removal of the max length of labels for those that are not mapped as it was done with Hermann and Cyril. |
Thanks @thomasbtnfr! I'll review this now. |
activity = {"@id": activity_id, | ||
"label": format_activity_name(common_prefix_act), | ||
"used": list(), | ||
"wasAssociatedWith": "RRID:SCR_007037", | ||
"wasAssociatedWith": "urn:" + agent_id, |
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.
@thomasbtnfr: Have you tried here to use directly a UUID (without the conversion to string)? I would like to check how the namespace is handled by the uuid library (rather than having to add manually the urn
namespace before).
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.
(If we add the urn namespace manually then we will have to declare it somewhere, in the context and in the RDF python library)
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.
Related to this issue : Inria-Empenn#13
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.
urn does not need to be defined in the context, it's handled appropriately as an uri by most libraries i believe.
https://api.dandiarchive.org/api/dandisets/000350/versions/0.221219.1506/assets/6b78d427-92e0-4a8e-bc64-481cbddea1e2/ (look at the publisher section). also we don't define it anywhere in our context: https://github.com/dandi/schema/blob/master/releases/0.6.3/context.json
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.
Thanks @satra! We opened Inria-Empenn#13 to work on this in a coming PR. I'll link your comment in there!
This PR solves the following issues: Inria-Empenn#6 Inria-Empenn#7 Inria-Empenn#8 and Inria-Empenn#9
Fork linked
[Bug]: Replace all niiri by urn Inria-Empenn/BEP028_BIDSprov#6 : all niiri prefixes have been replaced by urn
[Bug]: Use UUID as identifiers Inria-Empenn/BEP028_BIDSprov#7 : all @id identifiers have been replaced by uuid. Normally the links have not been broken especially with the following example :
[Bug]: Remove project info Inria-Empenn/BEP028_BIDSprov#8 : the project information has been removed
"@id": "exampleAgentID"
. Activities are directed to the agent by the agent id and not by its RRID.Moreover, the keywords "AssociatedWith" and "GeneratedWith" need to have "with" as a prefix to draw the graphs automatically. This prefix is not indicated in the standard (https://docs.google.com/document/d/1vw3VNDof5cecv2PkFp7Lw_pNUTUo8-m8V4SIdtGJVKs/edit#heading=h.h15lzmuljsky).