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

Fix: rdflib dumper ignores explicitly defined prefixes #348

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

sneakers-the-rat
Copy link
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

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
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.

3 participants