Skip to content
This repository was archived by the owner on Apr 16, 2026. It is now read-only.

Fix: rdflib dumper ignores explicitly defined prefixes - #348

Merged
cmungall merged 1 commit into
linkml:mainfrom
sneakers-the-rat:example-fix-dumper-prefixes
Jan 13, 2025
Merged

Fix: rdflib dumper ignores explicitly defined prefixes#348
cmungall merged 1 commit into
linkml:mainfrom
sneakers-the-rat:example-fix-dumper-prefixes

Conversation

@sneakers-the-rat

Copy link
Copy Markdown
Contributor

Related to: mapping-commons/sssom-py#558

copy and pasting discussion from slack:

  • first is that schemaview assigns from the default_curi_maps after explicitly defined prefixes - so those prefixes in the default curi map (which is the source of the allcaps HSAPDV) override explicit prefixes. explicit assignment should always override defaults
  • second is in the Namespaces object. currently it rejects repeated assignment to the same key. since it is a case insensitive dict, HsapDv is the same key as HASPDV . That shouldn’t be the case - explicit assignment should always override existing values
  • and then finally, as noted above, the rdflib dumper makes repeated calls to schemaview.namespaces() and tries to assign to it, which does nothing (or should do nothing, and if it does something then it’s a bug and a side effect of mutating values in the cache which 100% should not be relied on). So i instead made a single call to schemaview.namespaces(), assigning that to a variable, and then mutated that variable instead in the as_rdf_graph method.

opening this as a draft because i don't know this code, it fixes the linked issue, but i don't have a good sense of why things are the way they are currently.

@matentzn

Copy link
Copy Markdown

I just want to bump the importance of this PR for me as we make extensive use of RDFDumper in SSSOM py and right now, the outputs don't respect the provided prefix maps..

@cmungall
cmungall marked this pull request as ready for review November 15, 2024 20:51
@ticapix

ticapix commented Jan 13, 2025

Copy link
Copy Markdown

could we get it merged ?

@cmungall
cmungall merged commit c0d73b1 into linkml:main Jan 13, 2025
@sneakers-the-rat

Copy link
Copy Markdown
Contributor Author

Just as a note - this PR didnt have tests with it so would encourage ppl who are affected by this bug to follow on with tests to ensure against regression

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants