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

CurieNamespace catalog that is not a singleton and that uses curies #251

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

kervel
Copy link

@kervel kervel commented Mar 1, 2023

This PR introduces a new class CurieNamespaceCatalog that can convert between uries and curies using https://github.com/cthoyt/curies

Its main reason of existence is the detection of uri and curie synonyms in the given CurieNamespaces to create the converter in the correct way.

It is based on linkml #1244 with two modifications:

  • use of curies package
  • no use of classvars to store the catalog namespaces (so the catalog is not a singleton anymore)

This PR is needed to complete linkml/linkml#1257

after merging this PR, we will need to update poetry.lock (can't do it here or we will get merge conflicts)

hsolbrig and others added 2 commits March 23, 2023 12:11
It is my understanding that we are supposed to use the curies package, but it isn't clear how one
would add maps incrementally (see: tests/test_utils/test_curienamespace.py#113 as an example). Any
solution that passes test_curienamespace.py should do.
@kervel kervel force-pushed the curienamespace_index_catalogclass branch from 79a852e to 814b03e Compare March 23, 2023 11:12
@kervel kervel changed the title [draft] CurieNamespace catalog that is not a singleton and that uses curies CurieNamespace catalog that is not a singleton and that uses curies Mar 23, 2023
@cmungall cmungall requested a review from hsolbrig March 23, 2023 19:29
@codecov-commenter
Copy link

codecov-commenter commented Mar 24, 2023

Codecov Report

Merging #251 (814b03e) into main (b992fe0) will increase coverage by 0.61%.
The diff coverage is 70.70%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@            Coverage Diff             @@
##             main     #251      +/-   ##
==========================================
+ Coverage   63.54%   64.16%   +0.61%     
==========================================
  Files          53       53              
  Lines        6117     6284     +167     
  Branches     1650     1701      +51     
==========================================
+ Hits         3887     4032     +145     
+ Misses       1774     1770       -4     
- Partials      456      482      +26     
Impacted Files Coverage Δ
linkml_runtime/index/object_index.py 88.28% <12.50%> (-3.53%) ⬇️
linkml_runtime/dumpers/rdflib_dumper.py 94.94% <40.00%> (-2.97%) ⬇️
linkml_runtime/utils/context_utils.py 68.33% <50.00%> (-1.85%) ⬇️
linkml_runtime/utils/schemaview.py 83.29% <54.83%> (-3.11%) ⬇️
linkml_runtime/utils/curienamespace.py 93.44% <92.72%> (+8.82%) ⬆️
linkml_runtime/__init__.py 100.00% <100.00%> (ø)
linkml_runtime/utils/namespaces.py 73.37% <100.00%> (+2.48%) ⬆️
linkml_runtime/utils/yamlutils.py 79.44% <100.00%> (+1.29%) ⬆️

... and 1 file with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@kervel
Copy link
Author

kervel commented Mar 28, 2023

i just discovered we also have "namespacegen.py" which generates a python file that ... can be used to do expand/compress. so i have the impression that we have 100 ways to fix the "how to expand/compress uri's" problems.

so for an outsider it would be a bit strange to say look with namespacegen you can generate a Namespace object (which doesn't use the Namespaces class , which also can serve the same purpose btw), but if you then use pythongen we won't use that file to do exactly this.

so we have:

  • Namespaces class (now used in linkml) in linkml-runtime/utils/namespaces.py @hsolbrig seems you wrote a big part of that ? Couldn't we just use that one ?
  • The NamespaceCatalog proposed in this PR, which is basically now a convenience to create a CurieConverter without figuring out what is a synonym of what.
  • the output of linkml gen-namespaces.py which also generates a namespaces-like class, but the name is hardcoded to BiolinkNamespace

Update i tried to use Namespaces, it misses a few convenience functions: i have to deal with values that are uriorcurie which means they might be an uri or a curie. Namespaces doesn't have a function that goes from uriorcurie to uri or to curie, you have to know in advance if its an uri or a curie. I could add those instead of this PR.

@capsulecorplab
Copy link

capsulecorplab commented Jun 2, 2023

Should we perhaps open an issue for implementing a Namespace function that goes from uriorcurie to uri or curie?

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.

4 participants