Fix detection of CURIES vs. absolute paths on Windows #391
Conversation
dalito
force-pushed
the
issue2696-curie-vs-path-on-windows
branch
from
662b1ad to
a7a5102
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #391 +/- ##
==========================================
- Coverage 63.79% 63.76% -0.03%
==========================================
Files 63 63
Lines 8938 8942 +4
Branches 2584 2586 +2
==========================================
Hits 5702 5702
- Misses 2629 2631 +2
- Partials 607 609 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| @@ -73,7 +73,8 @@ def map_import(importmap: dict[str, str], namespaces: Callable[[None], "Namespac | |||
| else: | |||
| sname = os.path.join(expanded_prefix, lname) | |||
| sname = importmap.get(sname, sname) # Import map may use CURIE | |||
| sname = str(namespaces().uri_for(sname)) if ':' in sname else sname | |||
| if ':' in sname and not ":\\" in sname: # Don't interpret Windows paths as CURIEs | |||
There was a problem hiding this comment.
I know there's at least one other place (apart from the one below) where there's a similar check to try to determine if something is a CURIE or not. I wonder if it'd be better to extract this out into a separate function -- I know it's an extremely simple check, but maybe it would lessen the chance of similar errors occurring elsewhere. There's a bug in the SchemaViewer that stops remote schemas being imported from other remote schemas, and it's due to this.
There was a problem hiding this comment.
I am not sure if it is worth to move the check into a function. I still hope that one day we will use more specific types (Path, URI, CURIE) instead of string. This would make the code much cleaner.
There was a problem hiding this comment.
You're right, that's the best way to fix the issue. One day...!
|
Test added! ...and converted other tests in the file from unittest to pytest. I also made a corresponding PR for linkml: linkml/linkml#2698 which can only be merged after a new release of linkml-runtime with this fix. |
dalito
force-pushed
the
issue2696-curie-vs-path-on-windows
branch
from
cf714ab to
fa6d966
Compare
| @@ -103,7 +110,7 @@ def parse_import_map(map_: Optional[Union[str, dict[str, str], TextIOWrapper]], | |||
| if base: | |||
| outmap = dict() | |||
| for k, v in rval.items(): | |||
| if ':' not in v: | |||
| if ':' not in v or ":\\" in v: # Don't interpret Windows paths as CURIEs | |||
upstream_repo: dalito/linkml
upstream_branch: issue2696-remove-windows-special-casing
Fixes issues in
utils/context_utils.py:map_importfunction--all-extras(else the pandera tests fail)@ialarmedalien , @sierra-moxon - This was the problem you hit in linkml/linkml#2675
Related to linkml/linkml#2696