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

Removed some unnecessary RDF properties #87

Merged
merged 6 commits into from
Mar 18, 2021
Merged

Conversation

gaurav
Copy link
Member

@gaurav gaurav commented Mar 15, 2021

This PR removes some RDF properties from the JSON Schema:

  • While we need internalSpecifiers and externalSpecifiers properties in JSON, we probably don't need to include them in RDF (see Figure out what to do about the top-level object in a Phyx file #78, An object property for associating an internal specifier with a phyloreference phyloref-ontology#7, An object property for associating an external specifier with a phyloreference phyloref-ontology#8). If we change our mind later, we can add them back pretty easily.
  • We previously used the RDF version of the malformedPhyloreference field to let JPhyloRef know when a particular phyloreference was malformed (such as when it had no internal specifiers, or if one of its specifiers could not be mapped to a particular phylogeny node). JPhyloRef no longer checks this field at all, so we might as well remove it from here as well.
  • I've rewritten the component class system so that the two types of component classes are handled separately:
    • Direct subclasses are now linked to the phyloreference via the subClasses field (which is the @reverse of rdfs:subClassOf).
    • Cache component classes are now inserted directly into the logical expression when first used, and then subsequently referenced in other logical expressions as needed.
    • Component classes were previously stored and identified on a global basis (i.e. if #phyloref1 had 4 component classes, the first component class for #phyloref2 would be identified as #phyloref2_component5). I've changed this so that each phyloreference's component classes start counting from 1.

Closes #78.

While we need this in JSON, we probably don't need to include this in 
RDF (see #78). If we do need 
to insert it, we can add it back pretty easily.
We previously used this for letting JPhyloRef know when a phyloref was 
malformed, but it no longer checks this property -- it now just throws 
an error if reasoning fails.
@gaurav gaurav changed the title Removed testcase:internal_specifier and testcase:external_specifier Removed some unnecessary RDF properties Mar 15, 2021
@hlapp
Copy link
Member

hlapp commented Mar 15, 2021

BTW we might consider using phyloref:has_outside_TU for recording the external specifier, rather than phyloref:excludes_TU as phyloref/phyloref-ontology#8 indicated as conclusion. Or are we now getting rid of them altogether?

@gaurav
Copy link
Member Author

gaurav commented Mar 16, 2021

BTW we might consider using phyloref:has_outside_TU for recording the external specifier, rather than phyloref:excludes_TU as phyloref/phyloref-ontology#8 indicated as conclusion. Or are we now getting rid of them altogether?

internalSpecifiers/externalSpecifiers are not related to phyloref:excludes_TU/phyloref:has_outside_TU: the former is currently used to record the taxonomic units so that software looking only at the RDF can figure out what the internal and external specifiers for a particular phyloreference are, while the latter are part of the logical expressions we generate for phylorefs in OWL/JSON-LD. I think it's okay to get rid of them altogether, since any software that needs that information will probably be reading the OWL/JSON-LD file, which still has these fields (this is what the Open Tree Resolver does), so we don't really have a use-case for including it in the RDF yet.

Or perhaps you are suggesting that we record the internal and external specifiers using subclass statements, such as by describing the phyloref Alligatoridae as a subclass of includes_TU some ('Has Name' some (('Nomenclatural Code' value ICZN) and ('Name Complete' value "Alligator mississippiensis"))) and has_outside_TU some ('Has Name' some (('Nomenclatural Code' value ICZN) and ('Name Complete' value "Caiman crocodilus")))? That would work. I can open an issue to implement this if that would be useful.

I have opened an issue for converting the phyloref:excludes_TU to phyloref:has_outside_TU (#83). I didn't think that was a priority for the manuscript, but it shouldn't take too much work -- if it is a priority, go ahead and release a new version of the Phyloref Ontology and I'll make a PR to implement this.

@hlapp
Copy link
Member

hlapp commented Mar 17, 2021

Or perhaps you are suggesting that we record the internal and external specifiers using subclass statements, such as by describing the phyloref Alligatoridae as a subclass of includes_TU some ('Has Name' some (('Nomenclatural Code' value ICZN) and ('Name Complete' value "Alligator mississippiensis"))) and has_outside_TU some ('Has Name' some (('Nomenclatural Code' value ICZN) and ('Name Complete' value "Caiman crocodilus")))? That would work. I can open an issue to implement this if that would be useful.

We can discuss this at the next call, but I didn't think this what we wanted Phyx to look like, right? (As opposed to the generated OWL expressions)

@gaurav
Copy link
Member Author

gaurav commented Mar 17, 2021

We can discuss this at the next call, but I didn't think this what we wanted Phyx to look like, right? (As opposed to the generated OWL expressions)

Sounds good, I'll put it on the agenda!

@gaurav gaurav mentioned this pull request Mar 18, 2021
@gaurav gaurav marked this pull request as ready for review March 18, 2021 17:13
@gaurav gaurav requested a review from hlapp March 18, 2021 17:13
Copy link
Member

@hlapp hlapp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good as far as I understand 😉

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.

Figure out what to do about the top-level object in a Phyx file
2 participants