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

niiri prefixes > urn / id are now uuids / project info removed #83

Merged
merged 105 commits into from
Jan 12, 2023

Conversation

thomasbtnfr
Copy link
Collaborator

@thomasbtnfr thomasbtnfr commented Jan 9, 2023

This PR solves the following issues: Inria-Empenn#6 Inria-Empenn#7 Inria-Empenn#8 and Inria-Empenn#9

Fork linked

"wasGeneratedBy": {  
    "@id": "INRIA",  
    "@type": "Project",  
    "wasAssociatedWith": {  
      "@id": "NIH",  
      "@type": "Organization",  
      "hadRole": "Funding"  
    }

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).

hermann74 and others added 30 commits December 14, 2022 13:22
@cmaumet cmaumet changed the title Fix issues from fork #6, #7, #8, #9 niiri prefixes > urn / id are now uuids / project info removed Jan 11, 2023
@cmaumet
Copy link
Collaborator

cmaumet commented Jan 11, 2023

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 results and should be solved by running the exporter again).

@cmaumet
Copy link
Collaborator

cmaumet commented Jan 11, 2023

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:

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).

We should be able to correct for that by updating the context but let's do it in another iteration/PR.

@thomasbtnfr
Copy link
Collaborator Author

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.
For example :
the label in the jsonld of cfg_basicio.file_dir.file_ops.file_move (38 characters) now stays the same and does not become file_dir.file_ops.file_move (the limit was set to 30 characters).

@cmaumet
Copy link
Collaborator

cmaumet commented Jan 12, 2023

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,
Copy link
Collaborator

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).

Copy link
Collaborator

@cmaumet cmaumet Jan 12, 2023

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)

Copy link
Collaborator Author

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

Copy link
Collaborator

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

Copy link
Collaborator

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!

@cmaumet cmaumet merged commit 9f0fa8e into bids-standard:master Jan 12, 2023
@cmaumet cmaumet deleted the issues_6_7_8_9 branch January 12, 2023 13:34
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.

6 participants