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

Added: Antimicrobial Resistance topic #685

Merged
merged 4 commits into from
May 6, 2021

Conversation

AnkitaxPriya
Copy link
Contributor

This PR fixes issue #610

Summary

I have modified the EDAM_dev.owl file of edamontology repository.
I have added the topic of "Antimicrobial Resistance" in the EDAM_dev.owl file, keeping in view all the instructions mentioned in the CONTRIBUTING.md file to add a new concept.

@matuskalas kindly review my PR and let me know if any improvements are needed.
Thank you.

Co-authored-by: Mads Kierkegaard <mads.k@live.dk>
@AnkitaxPriya
Copy link
Contributor Author

@matuskalas I don't understand why all the checks have failed here, could you let me know where improvements are needed?

Copy link
Member

@matuskalas matuskalas left a comment

Choose a reason for hiding this comment

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

Very well done! 🙌🏽

Big thanks for adding the co-author correctly👍🏽 (you can see that GitHub recognises it).

There is just a couple of small things to fix:

  1. Can you find out from Travis CI why it fails? The error message is quite cryptic, and the line number is +1, but basically there is a small thing forgotten which makes it an invalid XML. I can give you a spoiler if you won't figure it out yourself, but maybe you'll be happier if you find it yourself 😺
  2. Please remove all the start and end spaces from the values of the first 6 XML elements inside the owl:Class
  3. I realised we could improve and extend the synonyms. I'd suggest these changes:
    • Removing the AR and ABR, these don't seem to be very obviously used abbreviations
    • Adding these 9 narrow synonyms, as follows, also with the abbreviations in brackets where stated:
      • Antifungal resistance
      • Antiviral resistance
      • Antiprotozoal resistance
      • Multiple drug resistance (MDR)
      • Multidrug resistance
      • Multiresistance
      • Extensive drug resistance (XDR)
      • Pandrug resistance (PDR)
      • Total drug resistance (TDR)

Million thanks! 🙏🏽

@AnkitaxPriya
Copy link
Contributor Author

  1. Can you find out from Travis CI why it fails? The error message is quite cryptic, and the line number is +1, but basically there is a small thing forgotten which makes it an invalid XML. I can give you a spoiler if you won't figure it out yourself, but maybe you'll be happier if you find it yourself 😺

I tried to sign up on their platform but cannot open it up. Kindly help.

  1. Please remove all the start and end spaces from the values of the first 6 XML elements inside the owl:Class

  2. I realised we could improve and extend the synonyms. I'd suggest these changes:

    • Removing the AR and ABR, these don't seem to be very obviously used abbreviations

    • Adding these 9 narrow synonyms, as follows, also with the abbreviations in brackets where stated:

      • Antifungal resistance
      • Antiviral resistance
      • Antiprotozoal resistance
      • Multiple drug resistance (MDR)
      • Multidrug resistance
      • Multiresistance
      • Extensive drug resistance (XDR)
      • Pandrug resistance (PDR)
      • Total drug resistance (TDR)

I've implemented the 2nd and 3rd changes. Added a screenshot here. Kindly let me know if any other changes are needed.

image

@matuskalas
Copy link
Member

I tried to sign up on their platform but cannot open it up. Kindly help.

You can view the "Details" (linking to https://travis-ci.org/github/edamontology/edamontology/builds/769487739) without any sign in.

I've implemented the 2nd and 3rd changes. Added a screenshot here. Kindly let me know if any other changes are needed.

Awesome, you're super fast 🐱‍🏍

Just please leave the <oboInOwl:hasExactSynonym>AMR... in, that one is useful.

image

This screenshot may suggest you where the error is 😉 (syntax highlighting works)

@matuskalas matuskalas added the concept/term addition Request for a new concept(s), or change(s) to existing concept(s) label May 5, 2021
Co-authored-by: Mads Kierkegaard <mads.k@live.dk>
@AnkitaxPriya
Copy link
Contributor Author

You can view the "Details" (linking to https://travis-ci.org/github/edamontology/edamontology/builds/769487739) without any sign in.

@matuskalas I understood what were the changes needed. I have amended it. 😄

Just please leave the <oboInOwl:hasExactSynonym>AMR... in, that one is useful.

I added oboInOwl:hasExactSynonym>AMR</oboInOwl:hasExactSynonym> in EDAM_dev.owl.

This screenshot may suggest you where the error is 😉 (syntax highlighting works)

Oh yes! The syntax highlight did make me realize my mistake.
Kindly review my PR and let me know if any other changes are needed to be done. 🙏

@AnkitaxPriya
Copy link
Contributor Author

@matuskalas It seems like my PR has only partially passed the checks.
From Travis CI I can observe that the test has failed preferably because of "Element ID" which has already been used in another ID. Also, it shows several conflicts w.r.t topic 'Microbiology'. How must I fix this? Please guide.

I've added the screenshot of the errors that showed up in Travis CI for reference.
image

@matuskalas
Copy link
Member

Awesome that you fixed all the previous problems! 🚀🙌🏽

From Travis CI I can observe that the test has failed preferably because of "Element ID" which has already been used in another ID. Also, it shows several conflicts w.r.t topic 'Microbiology'. How must I fix this? Please guide.

Now I can see that our validation works very well 😊. Can you find out from the error output what the problem could be? Hint: ID is in rdf:about

And could you please fix 2 more things:

  • I realised that we should perhaps add a second subClassOf, namely "Infectious disease". Can you find the ID/URI of that one? You can use the full-text search at https://bioportal.bioontology.org/ontologies/EDAM/?p=classes
  • Could you please move your whole owl:Class, including the <!--... line(!), a bit up inbetween the last Topic 4012 and the ObsoleteClass? With always 3 empty lines between the owl:Class-es (like elsewhere in the file). Please make sure to add the Co-authored-by: Mads Kierkegaard <mads.k@live.dk> again like you did in your previous commits👍🏽

Many thanks🙏🏽

@AnkitaxPriya
Copy link
Contributor Author

AnkitaxPriya commented May 6, 2021

Now I can see that our validation works very well 😊. Can you find out from the error output what the problem could be? Hint: ID is in rdf:about

Yes @matuskalas, I can very well locate that. I've added the ID value is 3301 but what value must I replace it with? Do I have to add +1 to the previous ID (i.e. 4012 + 1)?

And could you please fix 2 more things:

I cannot find the ID of "Infectious disease". I searched in the "Data" as well as "Topic" section. Kindly help me with finding the ID/URI. Many thanks.

@AnkitaxPriya
Copy link
Contributor Author

I cannot find the ID of "Infectious disease". I searched in the "Data" as well as "Topic" section. Kindly help me with finding the ID/URI. Many thanks.

Hello @matuskalas, with more exploration around the website I found the ID for "Infectious disease" i.e. http://purl.obolibrary.org/obo/IDO_0000436 😄

@matuskalas
Copy link
Member

Hello @matuskalas, with more exploration around the website I found the ID for "Infectious disease" i.e. http://purl.obolibrary.org/obo/IDO_0000436 😄

Nice😊 But please find one from EDAM. It will be in the form of http://edamontology.org/topic_... (Searching in the "Jump to" field at https://bioportal.bioontology.org/ontologies/EDAM/?p=classes will do the trick)

@AnkitaxPriya
Copy link
Contributor Author

Now I can see that our validation works very well 😊. Can you find out from the error output what the problem could be? Hint: ID is in rdf:about

Yes @matuskalas, I can very well locate that. I've added the ID value is 3301 but what value must I replace it with? Do I have to add +1 to the previous ID (i.e. 4012 + 1)?

Kindly let me know about how to fix this issue too @matuskalas.

@matuskalas
Copy link
Member

Yes @matuskalas, I can very well locate that. I've added the ID value is 3301 but what value must I replace it with? Do I have to add +1 to the previous ID (i.e. 4012 + 1)?

Bingo!🐱‍🏍 Please also update the value of the <next_id> in the beginning of the EDAM_dev.owl file accordingly.

Co-authored-by: Mads Kierkegaard <mads.k@live.dk>
@AnkitaxPriya
Copy link
Contributor Author

@matuskalas I have made the required changes. Seems like the Travis CI build is taking a lot of time. I hope that the commit doesn't have any conflict this time. 🤞
Well, even if it has any conflict I will be glad to resolve it. I am getting to learn a lot from all this. 😊

@AnkitaxPriya
Copy link
Contributor Author

AnkitaxPriya commented May 6, 2021

Hey @matuskalas, according to my observation from the results of the Travis CI build, I think I need to amend the following things:

  • I mistakenly mentioned two <rdfs:label>.., I need to remove one of them.
  • I've mentioned <rdfs:subClassOf rdf:resource="http://edamontology.org/topic_4013"/>, maybe I cannot mention the same ID in the subClassOf. Please correct me if I am wrong.

Please let me know if there are any other errors. I'd be happy to correct them all. 🙏

For now, I have made the mentioned changes in my local branch. Kindly check my attached screenshot and suggest other changes.

image

@matuskalas
Copy link
Member

matuskalas commented May 6, 2021

according to my observation from the results of the Travis CI build, I think I need to amend the following things

👍🏽

I mistakenly mentioned two <rdfs:label>.., I need to remove one of them.

Yes! Sorry I haven't seen it at first.

I've mentioned <rdfs:subClassOf rdf:resource="http://edamontology.org/topic_4013"/>, maybe I cannot mention the same ID in the subClassOf. Please correct me if I am wrong.

Exactly!
But please put back the "Microbiology" topic which was there before as the first of the subClassOf. In the end there should be 2 "subClassOf", one "Microbiology" and one "Infectious disease".

Please let me know if there are any other errors. I'd be happy to correct them all. 🙏

No more errors, but please fix 2 more things to make it PERFECT 😊

  • Could you please move your whole owl:Class, including the <!--... line(!), a bit up inbetween the last Topic 4012 and the ObsoleteClass? With always 3 empty lines between the owl:Class-es (like elsewhere in the file). Please make sure to add the Co-authored-by: Mads Kierkegaard <mads.k@live.dk> again like you did in your previous commits👍🏽
  • When you're done with everything else, please update the <oboOther:date> on the top of the file to the time when you're saving it, in UTC. Billion thanks! 🙇🏽‍♂️

Co-authored-by: Mads Kierkegaard <mads.k@live.dk>
@sonarcloud
Copy link

sonarcloud bot commented May 6, 2021

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@AnkitaxPriya
Copy link
Contributor Author

Yay! Finally, all the checks passed @matuskalas. 😄
I enjoyed making a PR. Kindly review it now.
I wish to resolve other such issues too.

All thanks to you for all the support and guidance, Matúš. 🙏

Copy link
Member

@matuskalas matuskalas left a comment

Choose a reason for hiding this comment

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

AWESOMEEEEEE!!! 😊

@matuskalas
Copy link
Member

Thank you so much for the great work @AnkitaxPriya!🙏🏽 Ready for being merged into main😸

@matuskalas matuskalas merged commit 5b03ef0 into edamontology:main May 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
concept/term addition Request for a new concept(s), or change(s) to existing concept(s)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants