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

EntityReference typedef: docs & validation inconsistent #512

Closed
joeflack4 opened this issue Apr 6, 2024 · 3 comments · Fixed by mapping-commons/sssom#358
Closed

EntityReference typedef: docs & validation inconsistent #512

joeflack4 opened this issue Apr 6, 2024 · 3 comments · Fixed by mapping-commons/sssom#358
Labels
bug Something isn't working

Comments

@joeflack4
Copy link
Collaborator

Overview

I'm running sssom validate and was getting an error that my creator_id was invalid. The docs for creator_id show that it is of type EntityReference, typeof: uriorcurie.

However, when I have a URI, creator_id: https://orcid.org/0000-0002-2906-7319, validation fails with:

  File "/Users/joeflack4/virtualenvs/icd11/lib/python3.10/site-packages/sssom/util.py", line 1319, in get_all_prefixes
    raise ValidationError(
jsonschema.exceptions.ValidationError: Slot 'creator_id' has an incorrect value: ['https://orcid.org/0000-0002-2906-7319']

If I change it to a CURIE, validation passes.

Suggested solution

a. Update the docs for EntityReference: typeof: curie.
b. Update get_all_prefixes() so that it ignores metadata slots where the typedef is not simply 'curie'. Or, if it's UriOrCurie, check to make sure at least one is valid.

Additional info

Example files

FYI: docstring issue

The docstring for get_all_prefixes() shows :raises ValidationError: If slot is wrong. twice.

@joeflack4 joeflack4 added the bug Something isn't working label Apr 6, 2024
@matentzn
Copy link
Collaborator

matentzn commented Apr 7, 2024

This is a confusing problem of linkml for me.. The type is really EntityReference, not uri_or_curie in the sense of "use one or the other". The correct way to express it would be: If its SSSOM TSV, or SSSOM JSON, it MUST be curie. If it is SSSOM RDF, it must be uri.

I guess this will have to be documented much much better.

@joeflack4
Copy link
Collaborator Author

Ah, that is not what I expected. That kind of conditional type def is tricky to express, but it also comes off like a code smell. Perhaps we could make EntityReference an abstract class and then subclass it, using different sub-types for JSON / TSV or RDF serialization, but IDK how that'd end up looking in the docs either.

Maybe can just update the description on it in the documentation to say exactly what you said:

If its SSSOM TSV, or SSSOM JSON, it MUST be curie. If it is SSSOM RDF, it must be uri.

@matentzn
Copy link
Collaborator

matentzn commented Apr 8, 2024

I made a PR about this: mapping-commons/sssom#358

We cant do anything else right now, so I will close this issue here, but I expect the PR to raise some eyebrows and subsequent dialogues.

@matentzn matentzn closed this as completed Apr 8, 2024
matentzn added a commit to mapping-commons/sssom that referenced this issue Apr 17, 2024
…358)

Resolves mapping-commons/sssom-py#512

- [X] `docs/` have been added/updated if necessary

The documentation for "entity references" is largely confusing, and
leading, in its current form, to the expectation that metadata fields
types with these can be _either_ a CURIE _or_ a full URI.

This is not quite right: in the TSV serialistion, as well as JSON, we
expect the values to be CURIE-strings. This PR documents this design
decision, but, as @joeflack4 puts it rightfully, this has a bit of a
"smell" to it. Not sure what the right thing is here, but this
documentation here is certainly better than not having it.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants