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

new health facility and other infrastructure terms #1105

Merged
merged 15 commits into from
Nov 12, 2021
Merged

Conversation

ddooley
Copy link
Contributor

@ddooley ddooley commented May 1, 2021

Re. Issue #1070
bridges, transport terms, utensils, food processing and agriculture infrastructure terms, some materials.
A redo of pull request 1071.

@ddooley
Copy link
Contributor Author

ddooley commented May 1, 2021

I believe the remaining warnings and errors are unrelated to this pull request.

@pbuttigieg
Copy link
Member

I believe the remaining warnings and errors are unrelated to this pull request.

Yes, these should be resolved as you just merged master into your branch - @turbomam figured out that the order of the arguments in the DOSDP command (the "generate" arg) was breaking things sometimes, randomly.

@turbomam
Copy link
Contributor

Shoot, I had intended to push a fix for the Makefile regarding the DOSDP arg order. @pbuttigieg good thing we didn't delete my merged branch yet. I'll see if I can dig that Makefile edit back up.

Basically, the DOSDP subcommand (like generate) should come before any named options.

@turbomam
Copy link
Contributor

Here's the DOSDP invocation that worked in my modified Makefile

@ddooley
Copy link
Contributor Author

ddooley commented May 19, 2021

I have updated the envo-edit.owl file, and removed a few stray top-level terms. There is an additional term import added to PATO and OBI import files; PATO one will take care of that top level 0040009 term. The other FoodOn top level numbered terms should disappear if ENVO were to do a new FoodOn import this weekend, and as well I've removed a few overbearing FoodOn equivalencies which may eliminate occurrence of some other top level FoodOn terms.

image

@pbuttigieg
Copy link
Member

Thanks @ddooley - we should do the import in this PR. If you plan a FOODON release this weekend, it makes sense to wait

@ddooley
Copy link
Contributor Author

ddooley commented May 20, 2021

I will try to finish FOODON release tomorrow!

@ddooley
Copy link
Contributor Author

ddooley commented May 24, 2021

The FoodOn v0.5 release has been done, which may help with some top-level stray ENVO terms on build.

@pbuttigieg
Copy link
Member

@ddooley - I don't see the FOODON import in this PR refreshed to match the new release. We'd need that to close this. Let me know if you'd do that or if you need backup.

@ddooley
Copy link
Contributor Author

ddooley commented Jun 10, 2021

@pbuttigieg I'm a little confused about what you said. Wasn't the plan to do a FoodOn release referesh separately from this pull request? If you meant to do a FoodOn release refresh within this pull request by running a build or a refreshed import on FoodOn import file via ontofox or something, let me know because yes I will need help to see how that is done in ENVO context.

@pbuttigieg
Copy link
Member

pbuttigieg commented Jun 14, 2021

@pbuttigieg I'm a little confused about what you said. Wasn't the plan to do a FoodOn release referesh separately from this pull request? If you meant to do a FoodOn release refresh within this pull request by running a build or a refreshed import on FoodOn import file via ontofox or something, let me know because yes I will need help to see how that is done in ENVO context.

Hi @ddooley

You're correct - the FOODON release is separate and on FOODON's side. Now that that's done, we'll re-run the make imports/foodon_import.owl target in our makefile to refresh the import for this PR.

The above follows this procedure: https://github.com/EnvironmentOntology/envo/wiki/Import-terms-from-other-ontologies

@pbuttigieg
Copy link
Member

The build failing is linked to a "too many requests" error here

@cmungall is there something up that we should be concerned about generally?

@ddooley
Copy link
Contributor Author

ddooley commented Jul 15, 2021

Any update on this? Is the June 14th issue resolved?

# Class: <http://purl.obolibrary.org/obo/ENVO_03501302> (cork (FOODON:03500046))

AnnotationAssertion(Annotation(<http://www.geneontology.org/formats/oboInOwl#hasDbXref> <https://en.wikipedia.org/wiki/Cork_(material)>) <http://purl.obolibrary.org/obo/IAO_0000115> <http://purl.obolibrary.org/obo/ENVO_03501302> "Plant matter consisting of an impermeable buoyant material, the phellem layer of bark tissue."@en)
AnnotationAssertion(<http://www.geneontol
Copy link
Member

Choose a reason for hiding this comment

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

this is an odd label

Copy link
Contributor Author

Choose a reason for hiding this comment

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

All (FOODON: ...) were accidental. Foodon id was meant just to be a reference to term to replace in foodon.

# Class: <http://purl.obolibrary.org/obo/ENVO_03501302> (cork (FOODON:03500046))

AnnotationAssertion(Annotation(<http://www.geneontology.org/formats/oboInOwl#hasDbXref> <https://en.wikipedia.org/wiki/Cork_(material)>) <http://purl.obolibrary.org/obo/IAO_0000115> <http://purl.obolibrary.org/obo/ENVO_03501302> "Plant matter consisting of an impermeable buoyant material, the phellem layer of bark tissue."@en)
AnnotationAssertion(<http://www.geneontol
Copy link
Member

Choose a reason for hiding this comment

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

I think subclassing CHEBI in ENVO should be an anti-pattern, and this should be an OBO anti-pattern more generally. I would make this an ENVO material composed-primarily-of something with chemical structure

But I think easiest to merge first then revisit later

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

# Class: <http://purl.obolibrary.org/obo/ENVO_03501302> (cork (FOODON:03500046))

AnnotationAssertion(Annotation(<http://www.geneontology.org/formats/oboInOwl#hasDbXref> <https://en.wikipedia.org/wiki/Cork_(material)>) <http://purl.obolibrary.org/obo/IAO_0000115> <http://purl.obolibrary.org/obo/ENVO_03501302> "Plant matter consisting of an impermeable buoyant material, the phellem layer of bark tissue."@en)
AnnotationAssertion(<http://www.geneontol
Copy link
Member

Choose a reason for hiding this comment

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

no

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member

@cmungall cmungall left a comment

Choose a reason for hiding this comment

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

Apologies for the delay

I am troubled by the lack of github actions, I guess this PR predated our use of them. Can you do a spurious git commit and push to trigger them?

I don't think NTRs to ENVO should ever subclass a non-ENVO class. Definitely no subclassing abstract BFO classes not slated for inclusion in COB. But we need to document this better on the ENVO side.

I am not totally sure ENVO is the best long term home for some of these super-specific concepts. However, as of now I do not have a concrete recommendation.

Overall I think we should merge ASAP and then address any issues en-masse as they likely infect other ENVO terms too

AnnotationAssertion(rdfs:label <http://purl.obolibrary.org/obo/ENVO_03501218> "toilet bowl"@en)
SubClassOf(<http://purl.obolibrary.org/obo/ENVO_03501218> <http://purl.obolibrary.org/obo/ENVO_01000989>)
SubClassOf(<http://purl.obolibrary.org/obo/ENVO_03501218> ObjectSomeValuesFrom(<http://purl.obolibrary.org/obo/BFO_0000050> <http://purl.obolibrary.org/obo/ENVO_01000516>))
SubClassOf(<http://purl.obolibrary.org/obo/ENVO_03501218> ObjectSomeValuesFrom(<http://purl.obolibrary.org/obo/RO_0000086> <http://purl.obolibrary.org/obo/PATO_0040009>))
Copy link
Member

Choose a reason for hiding this comment

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

true but useless. avoid axiomatizing anything not patternized

Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed pato quality

@cmungall
Copy link
Member

cmungall commented Nov 1, 2021

copied from other ticket:

@ddooley

Shall I make the label and parent changes on current pull request (which should trigger a new approval process?) > OR I'll make the changes asap on a new request after pull goes in?

@kaiiam:

I feel like it'd be better not to do the latter, but that's just me. I'd suggest either fixing the existing PR or regenerating a new one if that's easier. Whatever's easiest for you. Were you perchance using the > https://github.com/EnvironmentOntology/envo/wiki/ENVO-Robot-template-and-merge-workflow for a robot template of terms? Might help.

I agree with Kai, I think it's easier to tweak this PR but I am also OK with merging this then making a quick patch up merge, whatever works.

Also removed duplicate steel entry
Fixed ENVO terms under CHEBI classes
@ddooley
Copy link
Contributor Author

ddooley commented Nov 2, 2021

I've resolved problems with new terms. Apologies for label mishaps! Should be good to go.

About OBI device, I agree its definition indicates very narrow semantics for label. I'll ask on OBI list about whether label should be adjusted, and whether new general purpose device term added.

@kaiiam
Copy link
Contributor

kaiiam commented Nov 3, 2021

Quite the PR! Looking better in regard to the above comments about anti-patterns and labels. Definitions seem to follow ENVO conventions (I admit I don't have time to look though all of this in the depth it deserves). I agree with @cmungall's thoughts:

I am not totally sure ENVO is the best long term home for some of these super-specific concepts. However, as of now I do not have a concrete recommendation.

I think this could be the start of a whole new ontology of manufactured products and anthropogenic (human built) environments. Would that be better? Or should ENVO include terms like spoon and recreational cruise ship and medical clinic etc. I can see arguments both ways. Less ontologies means less re-importing of terms and technical issues, but having a separate ontology for something like manufactured products and human constructions would better delineate scope disciplines covered e.g., leaving earth and environmental sciences within ENVO. I understand anthropogenic environments have always been in ENVO's purview perhaps it would be manufactured products that should be housed elsewhere than ENVO?

Rought proposal idea: Manufactured Product Ontology (MPO) could be the home for the current ENVO:00003074 manufactured product hierarchy and be responsible for all variations of spoons and squeegees and other endless crap (pardon my language) that humans produce. Meanwhile ENVO could remain the host for all environments natural and anthropogenic and be the home for terms like processing plant and hospital unit facility etc. Just a suggestion the alternative being that ENVO absorbs all of this.

Thoughts?

@ddooley
Copy link
Contributor Author

ddooley commented Nov 3, 2021

Support for MPO probably depends on curation communities that need it directly. For us at CIDGOH.ca and people like Cornell food safety research folks we need to name the manufactured products (and their guts) as they relates to disease vectors / pathogen contamination / sample sites, so we would be an active contributor. But there are so many other uses for same vocab, including behavioural science / consumer behaviour etc. I think IOF (Industrial Ontology Foundry) would be a keen supporter too; so far I'm not hearing from them about creating something like MPO directly.

@kaiiam
Copy link
Contributor

kaiiam commented Nov 3, 2021

It's just an idea although @ddooley @matentzn and I have bounced it around rhetorically. Just something to consider I'm obviously in favor and willing to help set it up, but only if people think it's a useful thing for OBO overall.

@pbuttigieg
Copy link
Member

I'm fine with merging in this in and tweaking later. There are multiple issues to discuss here and also some policy questions to consider.

@kaiiam
Copy link
Contributor

kaiiam commented Nov 12, 2021

Sounds good. If in the future it makes sense to scope the manufacture product hierarchy elsewhere we could deal with that then. For now ENVO will house those terms.

@pbuttigieg
Copy link
Member

Sounds good. If in the future it makes sense to scope the manufacture product hierarchy elsewhere we could deal with that then. For now ENVO will house those terms.

Yes, and I wouldn't move them until there's a viable ontology up and running

@pbuttigieg pbuttigieg merged commit 9af2c9e into master Nov 12, 2021

# Class: <http://purl.obolibrary.org/obo/ENVO_03501102> (polytunnel)

AnnotationAssertion(Annotation(<http://www.geneontology.org/formats/oboInOwl#hasDbXref> <http://purl.obolibrary.org/obo/ENVO_>) <http://purl.obolibrary.org/obo/IAO_0000115> <http://purl.obolibrary.org/obo/ENVO_03501102> "A tunnel made from steel and covered in polyethylene used for plant cultivation."@en)
Copy link
Member

Choose a reason for hiding this comment

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

How did this happen?

Copy link
Member

@cmungall cmungall left a comment

Choose a reason for hiding this comment

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

there are multiple syntactic issues. Not sure why this did not get caught


# Class: <http://purl.obolibrary.org/obo/ENVO_03501109> (subway train)

AnnotationAssertion(Annotation(<http://www.geneontology.org/formats/oboInOwl#hasDbXref> <http://purl.obolibrary.org/obo/ENVO_>) <http://purl.obolibrary.org/obo/IAO_0000115> <http://purl.obolibrary.org/obo/ENVO_03501109> "A passenger train which runs along an undergroud rail system."@en)
Copy link
Member

Choose a reason for hiding this comment

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

multiple cases of this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

On another thread I mentioned that this was about our adding hasDbXref "ENVO" to mark the definition source but that robot converted this into URL http://purl.obolibrary.org/obo/ENVO_ and I didn't notice that weirdness. I can go in and delete all these manually. But does anyone want to mark that envo curators were the source of the definition a different way?

Copy link
Collaborator

Choose a reason for hiding this comment

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

@ddooley I think it is probably best to use oboInOwl:source (or dc:source), whatever ENVO uses, instead of hasDBXref..

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If I'm not mistaken ENVO uses hasdbxref as annotation on definitions to point to definition source. (Personally, given that a term should have only one definition I have been fine with the IAO:0000118 definition source annotation on the term itself that other projects use). At any rate I did a pull request #1309 to get rid of the bad hasDBXrefs.

Copy link
Contributor

Choose a reason for hiding this comment

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

ENVO uses hasdbxref as annotation on definitions to point to definition source.

Yes that's our standard, theres always more cleanup but thanks @ddooley for cleaning up in #1309 much obliged!

@kaiiam
Copy link
Contributor

kaiiam commented Mar 31, 2022

Yes we should probably fix all this before the next release.

@pbuttigieg pbuttigieg deleted the issue-1070-v2 branch August 15, 2022 16:22
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.

6 participants