-
Notifications
You must be signed in to change notification settings - Fork 0
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
Adds a phyx2ontology.js script for building a synthesized Clade Ontology #58
Conversation
Ignore this. It's a logic consequence of the model in this concrete case. |
This seems to work for one external specifier, but fails for more.
Just to be clear, this PR is not urgent, and will probably need a detailed review. Feel free to review it whenever you have the time! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is large enough that I'm not exactly sure what to specifically look over (and it's too much to review everything). I guess there's nothing that stands out big as an issue.
The one thing I might ask is how are we testing this script, i.e., that the code in this script is performing its actions correctly according to expectations. It seems the tests are all at a downstream level, and don't directly test this script. It seems this script complex enough that to facilitate maintainability, it should come with its own set of tests.
Good point! I've added a very basic test that just runs phyx2ontology.js and makes sure that it produces valid JSON output. I think that's good enough for this PR, and then #65 covers adding shape-based testing of the produced JSON-LD output. What do you think? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me.
This PR updates phyx.js to produce Model 2.0 ontologies (closes #4). It updates TaxonomicUnitWrapper, TaxonConceptWrapper, TaxonNameWrapper and SpecimenWrapper to produce equivalent class expressions for taxonomic units. These are used by PhylogenyWrapper to produce OWL restrictions for nodes, which are added to their `rdf:type` in the form `cdao:represents_TU some [taxonomic unit as equivalent class expression]`. It also completely replaces the code used to generate JSON-LD from a phyloreference in PhylorefWrapper with the model 2.0 code from the Clade Ontology (phyloref/clade-ontology#58). Note that this code needs better automated testing (see phyloref/clade-ontology#67 for an example). Finally, we previously modeled specifiers as containing one or more taxonomic units. This PR updates the model so that specifiers *are* taxonomic units. Making these changes has lead to the creation of a new Phyx context file. This allowed us to start changing properties to bring them in line with the new version of the Phyloref ontology (closes #9, #10). Note that we still use the model 1.0 method for writing down which phyloreferences are expected to resolve to particular node, i.e. by labeling the node with the phyloreference or by using the `testcase:expected_phyloreference_named` property. Implementing that will require updating JPhyloRef to use the model 2.0 method as well, and so I'll do that in a separate PR.
This PR adds a script that can be used to create a synthesized Clade Ontology from the Phyx files in this repository. It currently generates an OWL 2 DL JSON-LD representation using phyx.js, and then converts it into a OWL 2 EL JSON-LD representation on the fly. I will eventually move this code into phyx.js, but only after JPhyloRef has been updated to be able to test OWL files using OWL 2 EL reasoners.
WIP -- this code is still very quick-and-dirty and will need a bunch of cleanup before it's ready for review. Also, there are still some bugs in the output -- for example, "Tomistominae (alt)" resolves to two nodes on the same phylogeny, which should be impossible -- so these should be fixed before this PR is reviewed.
Should be merged after PR #60 has been merged.