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

Release 2022 03 31 (easier review) #2376

Merged
merged 11 commits into from
Apr 6, 2022
Merged

Release 2022 03 31 (easier review) #2376

merged 11 commits into from
Apr 6, 2022

Conversation

matentzn
Copy link
Contributor

@matentzn matentzn commented Apr 1, 2022

No description provided.

@matentzn matentzn requested a review from shawntanzk April 1, 2022 09:31
@shawntanzk
Copy link
Collaborator

removed anatomical entity doesn seem a big deal, as they have is_a axioms to something that is basically an anatomical entity, im guessing that this is because of the changes in bridge equivalence to caro - don't think its a huge deal

@shawntanzk
Copy link
Collaborator

Copying this over:

Things I would like to get signed off/source found before committing to the release:

  1. The whole lot of epithelium related stuff that are now just anatomical entity, heaps of changes with this, would like to know the source of the changes
  2. The massive removal of neural crest related axioms
  3. The huge removal of subclassof anatomical entity (they do seem to be anatomical entities so not sure why this is happening)

@shawntanzk
Copy link
Collaborator

for the epithelium stuff, it might be this: 5a65039 in which case it should be fine, but its weird that it didnt show up in the logical diff?

@shawntanzk
Copy link
Collaborator

might be of concern, left is this branch, right is last release - GCIs seem to be removed
Screenshot 2022-04-01 at 16 33 12

@shawntanzk
Copy link
Collaborator

I'm looking at a few classes with 'structure with developmental contribution from neural crest' - and it seems that they can be inferred some other way, so it makes sense that those axioms are removed. The question I have is why is it happening this time?

@matentzn
Copy link
Contributor Author

matentzn commented Apr 1, 2022

This is super weird! So many GCIs removed does not smell good. Can we somehow figure out which file they were supposed to be in? originally? (The GCIs cannot be in Uberon..). Is it possible they came back through some import in the past?

@shawntanzk
Copy link
Collaborator

This is super weird! So many GCIs removed does not smell good. Can we somehow figure out which file they were supposed to be in? originally? (The GCIs cannot be in Uberon..). Is it possible they came back through some import in the past?

oh that explains it, sorry lol, was using the edit file, should probably have used the built uberon >.< just checked all good

@shawntanzk
Copy link
Collaborator

left to do: see if epithelium stuff is also just removing things that can be inferred

@shawntanzk
Copy link
Collaborator

It seems like this new release removed many axioms that can technically be inferred:
Screenshot 2022-04-04 at 08 17 35
Screenshot 2022-04-04 at 08 18 42

I'm not sure why this is done as the inferences are pretty complex (like you need to know what you are looking for for that) Need input on how to proceed.

@shawntanzk
Copy link
Collaborator

Potential reason for massive changes: obophenotype/cell-ontology#1532

@balhoff
Copy link
Member

balhoff commented Apr 4, 2022

These property chains were removed on purpose: #1808

@shawntanzk
Copy link
Collaborator

strange that the changes are only showing up in this release? I'm guessing because of all the import dependencies, it took awhile to finally be clean?

@balhoff
Copy link
Member

balhoff commented Apr 4, 2022

I agree, it sure took a while! But the imports have been a tangle. There are some axioms that only live in imports and get passed back and forth.

@matentzn
Copy link
Contributor Author

matentzn commented Apr 6, 2022

@shawntanzk can you do the final review here and approve!

Copy link
Contributor Author

@matentzn matentzn left a comment

Choose a reason for hiding this comment

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

I approve this!

@matentzn
Copy link
Contributor Author

matentzn commented Apr 6, 2022

For future reference, this is yet undadressed: #2382

@matentzn matentzn merged commit cac2b9b into master Apr 6, 2022
@matentzn matentzn deleted the release-2022-03-31 branch April 6, 2022 12:49
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.

3 participants