.entities() typedef update
#688
Conversation
| @@ -97,7 +97,7 @@ def label(self, curie: CURIE, lang: Optional[LANGUAGE_TAG] = None) -> str: | |||
| else: | |||
| raise ValueError(f"Label must be literal, not {label}") | |||
|
|
|||
| def entities(self, filter_obsoletes=True, owl_type=None) -> Iterable[CURIE]: | |||
| def entities(self, filter_obsoletes=True, owl_type=None) -> Iterable[Union[URI, CURIE]]: | |||
There was a problem hiding this comment.
The update
This is a minor update. I just happened to notice that while I was using this for SqlImplementation, I was getting back URIs sometimes. I assume this should always be a possible thing for the other implementations as well.
| @@ -97,7 +97,7 @@ def label(self, curie: CURIE, lang: Optional[LANGUAGE_TAG] = None) -> str: | |||
| else: | |||
| raise ValueError(f"Label must be literal, not {label}") | |||
|
|
|||
| def entities(self, filter_obsoletes=True, owl_type=None) -> Iterable[CURIE]: | |||
| def entities(self, filter_obsoletes=True, owl_type=None) -> Iterable[Union[URI, CURIE]]: | |||
There was a problem hiding this comment.
Alternatives?
LinkML has URIorCURIE but I don't know if it's something that should be used here.
There was a problem hiding this comment.
No The URI and CURIE definition are oaklib specific. So the sis correct.
| @@ -97,7 +97,7 @@ def label(self, curie: CURIE, lang: Optional[LANGUAGE_TAG] = None) -> str: | |||
| else: | |||
| raise ValueError(f"Label must be literal, not {label}") | |||
|
|
|||
| def entities(self, filter_obsoletes=True, owl_type=None) -> Iterable[CURIE]: | |||
| def entities(self, filter_obsoletes=True, owl_type=None) -> Iterable[Union[URI, CURIE]]: | |||
There was a problem hiding this comment.
Order of types?
This is a super minor thing, but I am going to guess that if there is a Python convention, maybe it should be Union[CURIE, URI] for alphabetical reasons, but I hose Union[URI, CURIE] because that's usually the order we speak them in the semantic web world.
joeflack4
force-pushed
the
entities-typedef
branch
from
80ff45c
to
5751fd4
Compare
… previously just Iterable[CURIE], but it is also possible for it to return URIs.
joeflack4
force-pushed
the
entities-typedef
branch
from
5751fd4
to
7e667d8
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## main #688 +/- ##
=======================================
Coverage 76.50% 76.50%
=======================================
Files 252 252
Lines 29368 29368
=======================================
Hits 22468 22468
Misses 6900 6900 ☔ View full report in Codecov by Sentry. |
|
Hi @joeflack4 - it's best to discuss what you want to do in an issue first. The intent is that CURIEs are input and output. Yes, I know that when using semsql URIs that have no prefixes get turned into quoted URIs (different from URIs), but this should be fixed elsewhere. Is this a blocker for you? |
|
@cmungall Not a blocker. Just thought it was a quick way to contribute. Agreed it's generally good practice to make an issue. I thought this one would be an easy merge and not merit that, but it looks like this does require discussion. Here are my thoughts (also added to the issue). Details
I guess you're right, they are "quoted URIs". Here's an example of one I see:
Why don't we add something like QUOTED_URI to CURIE = str
QUOTED_URI = str
URI = strWhen you say "should be fixed elsewhere", I assume you are referring to them being "quoted" URI strings rather than plain URI strings. I'm definitely down with that. If you mean that URIs should never be returned (you probably don't), I don't think that's possible 100% of the time. |
Updates