-
Notifications
You must be signed in to change notification settings - Fork 4
Better support for external links #54
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
Conversation
…the local Collection) - Link objects are now properly serialized in JSON-LD - Links can now be annotated with a list of types that they may be pointing to - this allows collections that contain Links to validate - Link is now directly available in the top-level openminds module.
|
I'm unsure whether |
| from openminds.v4.controlled_terms import Species | ||
| from openminds.v4.core import DatasetVersion | ||
|
|
||
| maybe_mouse = Link("https://openminds.om-i.org/instances/species/musMusculus") | ||
|
|
||
| definitely_mouse = Link( | ||
| "https://openminds.om-i.org/instances/species/musMusculus", | ||
| allowed_types=[Species], | ||
| ) |
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.
It would be better to avoid hardcoding the version and namespace of the instances.
I would also move:
"definitely_mouse = Link(
"https://openminds.om-i.org/instances/species/musMusculus",
allowed_types=[Species],
)"
just on top of "my_dsv2 = DatasetVersion(study_targets=[definitely_mouse])"
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.
"better to avoid hardcoding" - do you mean the test should iterate over all versions, and introspect the namespace from the version?
"would also move" - I think this is a matter of taste. I think that by grouping the Link creation on consecutive lines it makes the test easier to understand.
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.
Yes to the first option, because it will require to be adjusted for each new version (v5.0 and above).
The second point seems fair.
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.
It doesn't need to be adjusted for new versions, the test would work with any openminds version, I chose v4 for no particular reason.
(i.e., to online instances outside the local Collection)
Example:
(this is perhaps a bad example, since all instances in the openMINDS instances library are anyway directly available (e.g.
mouse = Species.mus_musculus, but it serves to illustrate the idea)