search

linkml / linkml-runtime

Runtime support for linkml generated models
https://linkml.io/linkml/
Creative Commons Zero v1.0 Universal
24 stars 23 forks source link

Improve curie validation #285

Open Silvanoc opened 11 months ago

Silvanoc commented 11 months ago

Removing wrong expectations on CURIEs and adding new expectations. Also fixing validation according the specification.

codecov[bot] commented 11 months ago

Codecov Report

Attention: 2 lines in your changes are missing coverage. Please review.

Comparison is base (d45970a) 62.09% compared to head (cf960f0) 62.08%. Report is 1 commits behind head on main.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #285 +/- ## ========================================== - Coverage 62.09% 62.08% -0.01% ========================================== Files 63 63 Lines 8461 8459 -2 Branches 2170 2169 -1 ========================================== - Hits 5254 5252 -2 Misses 2599 2599 Partials 608 608 ``` | [Files](https://app.codecov.io/gh/linkml/linkml-runtime/pull/285?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=linkml) | Coverage Δ | | |---|---|---| | [linkml\_runtime/utils/metamodelcore.py](https://app.codecov.io/gh/linkml/linkml-runtime/pull/285?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=linkml#diff-bGlua21sX3J1bnRpbWUvdXRpbHMvbWV0YW1vZGVsY29yZS5weQ==) | `78.26% <50.00%> (-0.18%)` | :arrow_down: |

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

cmungall commented 8 months ago

You you still working on this PR @Silvanoc ?

Silvanoc commented 8 months ago

You you still working on this PR @Silvanoc ?

Sorry, I completely forgot this PR. Ich had a private discussion on Slack with @sierra-moxon, where she was raising her concern about the convenience of the changes implying that something like 17 would become an accepted CURIE (see the tests).

I'm quoting here her concern and my answer:

By @sierra-moxon:

Hi again - I see you opened another draft PR on this -- and are currently validating that 17 is a valid curie? I don't think that is a valid curie (I don't claim to be an expert here, but if I had data that used 17 as an identifier, I would want a validation exception generated).

By @Silvanoc:

If I've read the spec right, 17 is a valid CURIE as abc. If LinkML should only support a subset of the spec-conform CURIEs, then it probably should be specified within the project. My feeling is that you expect at least the colon : and possibly also the prefix. We can of course have validation rules for the LinkML CURIE subset. But as I've said, we should first have a clear specification. We could have something like "LinkML CURIEs are all valid CURIEs according the W3C specification providing both a prefix and the separation colon".

After that I simply forgot about it.

@cmungall what's your opinion about it? Could you formulate it with comments on the tests? Something like "I wouldn't expect this to be accepted" or otherwise.