ropensci / RNeXML

Implementing semantically rich NeXML I/O in R
https://docs.ropensci.org/RNeXML
Other
13 stars 9 forks source link

Fixes CDAO namespace definition #219

Closed hlapp closed 5 years ago

hlapp commented 5 years ago

Also changes the CDAO prefix in example files that still use the old (1.0) version of the ontology, so that it doesn't clash with the namespaces defined in nexml_namespaces.

In an ideal world this shouldn't matter - the namespaces defined in a NeXML file should simply take precedence. In practice the "nexml" object is initialized with a predefined list of namespaces, for which there are good reasons (programming friendliness, for example), and when adding namespaces those with an already existing prefix get skipped. As a result, if reading a file that defines xmlns:cdao with the v1.0 URI, the resulting object will move those to the post-1.0 URI, where the terms don't exist (because post-1.0 term identifiers use OBO-style opaque identifiers). (Note that this doesn't break reading, writing, or even only the syntactic validity of the term URIs -- they won't resolve but for RDF URIs don't have to resolve. But what's the point of all this if don't put value in faithfully retaining the full URI identifiers of all ontology terms and properties.)

Perhaps a better, or useful in addition, fix would be to reset the initialized list of namespaces to an empty list when converting from XML, and then adding our custom list at the end. In that order, the prefix expansion that was in the NeXML file would now take precedence over our custom list.

Thoughts? The more I think about it, the latter seems advised regardless of changing the ciao: prefix in data files, because otherwise there's a risk that the namespaces defined for the "nexml" object aren't the same expansions as those defined i the NeXML file that was read.

hlapp commented 5 years ago

I decided to go with the latter idea, i.e., not change data files but change the code to make it safe to deal with files that use some of the prefixes in a different way. This is now tested for, too. (It turns out there was meant to be a test, judging by a comment, but it was never written, so no failure previously 😳

cboettig commented 5 years ago

yes, I like the latter idea better too, makes sense to me. merge when ready! :shipit: