Fixes #1801 NTR placental blood

[ghost]

Fixes #1801 NTR placental blood

[ghost]

@paolaroncaglia, I will appreciate your review of this new term.
There appear to be several artefacts (lines dropped and added). I'm not sure why these appear and have seen similar occurrences with other PRs.

[paolaroncaglia]

@bvarner-ebi

@paolaroncaglia, I will appreciate your review of this new term.
There appear to be several artefacts (lines dropped and added). I'm not sure why these appear and have seen similar occurrences with other PRs.

Nothing like a seemingly simple ontology edit to stir a little ontology conversation :-D

• Shawn and Nico are running a Uberon release. Last week Nico suggested I hold on to committing new edits until after the release. So, to be on the safe side, I won't approve this one for now unless/until they give the go-ahead.
• The line dropping and adding, including addition of imported term labels (e.g. FMA labels in this case), is an artefact due to OBO/Protege serialisation, if I remember correctly. It can be ignored provided one double-checks that nothing has changed de facto. In this case, we've both checked and it's all fine.
• You made 'placental blood' an
  intersection_of: UBERON:0000178 ! blood
  intersection_of: located_in UBERON:0001987 ! placenta
  which I agree with. The existing sibling 'umbilical cord blood' is modelled in the same way. However, the other 3 existing children of 'blood', i.e. arterial/capillary/venous blood, presumably added at the same time and by the same editor based on their IDs, are 'part of' not 'located in'. I think the part of is incorrect and should be changed. 'part of' = "a core relation that holds between a part and its whole", while 'located in' = "a relation between two independent continuants, the target and the location, in which the target is entirely within the location". The ID range of those 3 terms falls into Chris Mungall's, so we might want to check before changing. As I said, I think your new term is fine. Thanks. (Quick addition: the choice of one relationship vs. the other may have implications on reasoning depending on different "settings" etc.)

[paolaroncaglia]

Commented on ticket

[ghost]

@paolaroncaglia, as always, thank you for the thorough instruction.

Well noted on pausing this merge until @shawntanzk or @matentzn confirm the new release.

@cmungall, would you like to offer feedback on Paola's last point in her comment above?

[shawntanzk]

Hey sorry about the release, hunting down some really difficult to find changes/reasons, but we might have found it in the tech call. Will keep you all updated.

[ghost]

Hi @shawntanzk, @matentzn, it looks like the new release was published. Can you confirm we may proceed with merging this PR?

[shawntanzk]

@matentzn - will you do a full release? The last release version only had uberon.obo changed right?

[matentzn]

Nope, all updated!

[matentzn]

Scroll down: https://github.com/obophenotype/uberon/releases/tag/v2022-04-05

[ghost]

@paolaroncaglia, now that we may proceed, would you kindly approve this PR?

[ghost]

@matentzn, @shawntanzk, tapping your expertise.... can you tell why this latest commit failed?

[matentzn]

You have a lot of diff for no reason that I can see. Are you using Protege 5.5?

[ghost]

Thanks for reviewing, @matentzn.

Yes, using Protégé 5.5.0.

@paolaroncaglia explained the lines dropping and adding in her comment above (point 2).

I've been adding my ORCiD as a contributor annotation straight to the terms I create. I've been adding the requester's ORCiD as a definition annotation. I switched the latter from contributor to xref. However, the QC is still failing....

[matentzn]

@bvarner-ebi do you have ODK installed? If so, can you try to run:

cd src/ontology
sh run.sh make reports/uberon-edit-xp-check IMP=false PAT=false BRI=false MIR=false

If you don't, you can nudge @shawntanzk to do so.

@bvarner-ebi note that in the error report, you should look at the next item:

In this case it does not help, but usually it does :P sorry.

@shawntanzk can you try to hack the uberon.Makefile so that:

instead of

...reports/uberon-edit-xp-check 2> reports/uberon-edit-xp-check.err || (echo "problems" && exit 1)

(in the reports/uberon-edit-xp-check goal)

we run something like:

...reports/uberon-edit-xp-check 2> reports/uberon-edit-xp-check.err || (echo "problems" && tail reports/uberon-edit-xp-check.err && exit 1)

? try locally as above.

[paolaroncaglia]

@bvarner-ebi

@paolaroncaglia, now that we may proceed, would you kindly approve this PR?

I know the checks are still failing, but I approved the content changes. Thanks.

[ghost]

@matentzn, I ran the command you suggested and see it generated uberon-edit-xp-check.err.

It has a few flags stating:
'FLAG: relation used but not defined'

An example:
"FLAG: relation used but not defined
'IAO:0000700' -- property_value: IAO:0000700 UBERON:0000104
property_value: IAO:0000700 UBERON:0001062"

Are these flags the reason QC is failing? I did not make the edits mentioned in the flags, so if they are the reason, how did this previously pass QC?

[matentzn]

You were working of an old branch.. I pushed now changes from master to your branch, and it should pass!

[ghost]

@matentzn - is this the way to do it for CL as well? I updated my Protégé preferences.

[matentzn]

If it is not, it should be!

[ghost]

Will add to the next CL meeting agenda so everyone is on the same page (although it just may be myself who had this setting!).

[ghost]

See https://oboacademy.github.io/obook/lesson/contributing-to-obo-ontologies/#setup under section "Setting Preferences for New entities metadata". See the screenshot. Should this be updated or is it specific to Mondo?

[matentzn]

I put it on the agenda for the workshop on Tuesday.