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

Map MS partof to RO #232

Merged
merged 2 commits into from
Jan 24, 2024
Merged

Map MS partof to RO #232

merged 2 commits into from
Jan 24, 2024

Conversation

matentzn
Copy link
Contributor

Fixes EBISPOT/ols4#600

Ideally, in a future PR, you should consider to import RO rather than copy pasting the semantics here (i.e. transitivity).

@ypriverol ypriverol self-requested a review January 24, 2024 10:15
@@ -45,6 +45,7 @@ name: has_units
[Typedef]
id: part_of
name: part_of
xref: BFO:0000050
Copy link
Contributor

Choose a reason for hiding this comment

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

@matentzn Can you update the version of the ontology. See the failing Actions https://github.com/HUPO-PSI/psi-ms-CV/actions/runs/7638645173/job/20810093289

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Feel free to close this PR and redo it, I just wanted to show you how you can map your part of to RO, so it displays correctly in OLS and interoperates with other ontologies. You can also directly edit my PR and make the required changes!

Copy link
Contributor

Choose a reason for hiding this comment

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

Done. Thanks a lot for the tip.

@ypriverol ypriverol self-requested a review January 24, 2024 10:17
@ypriverol ypriverol requested a review from edeutsch January 24, 2024 10:33
Copy link
Contributor

@edeutsch edeutsch left a comment

Choose a reason for hiding this comment

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

It would be funny if after all this pain, the solution is as simple as this..

@ypriverol ypriverol added this pull request to the merge queue Jan 24, 2024
Merged via the queue into HUPO-PSI:master with commit 804d6e9 Jan 24, 2024
2 checks passed
@ypriverol
Copy link
Contributor

I merged the PR to see the changes and opened the owl created in Protege as suggested; and it looks for me that the problem still remains

Screenshot 2024-01-24 at 20 17 51

Any advice @matentzn ?

@matentzn
Copy link
Contributor Author

This is a weakness in Protege I think. If you look at the peptide classes, you can see peptide attribute underneath, with the blue arrow..

image

Lets see how Protege will display this..

@matentzn
Copy link
Contributor Author

(I think the problem in Protege is that it will include the is a hierarchy no matter what, and superimposes the other "relation ships" after the fact - which means that what you see in terms of top level roots are all the roots according to the is a hierarchy)

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.

MS ontology - Unexpected behaviour/problems with the display in OLS4
3 participants