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

merge CL:0002294 into CL:0009077 #2110

Closed
wants to merge 3 commits into from
Closed

merge CL:0002294 into CL:0009077 #2110

wants to merge 3 commits into from

Conversation

shawntanzk
Copy link
Contributor

@shawntanzk shawntanzk commented Aug 18, 2023

Fixes #2101
Requires changes in Uberon (changing UBERON:0006936 to use CL:0009077)
My choice of choosing CL:0009077 was due to its use in HRA subset
I see that there are obsoletion notices now - not sure exact ways of working now so will leave that to @bvarner-ebi
Also due to obsoletion method, import file is changed - happy for this change to be reverted and fixed at source (Uberon).
PS could someone help me fix it in Uberon side please? requires import and its a whole thing here.

Thanks!

@shawntanzk shawntanzk requested review from emquardokus and a user August 18, 2023 06:58
@shawntanzk shawntanzk self-assigned this Aug 18, 2023
@ghost ghost removed their request for review August 18, 2023 07:36
@ghost
Copy link

ghost commented Aug 18, 2023

@shawntanzk, thanks for the suggested changes.
If you propose to merge/obsolete a term, kindly just open an issue. An editor will review and make an announcement on slack and give at least 2 weeks' notice before obsoleting if there is no objection. We're requesting outside collaborators to request PR reviewers instead of automatically assigning them.

@shawntanzk
Copy link
Contributor Author

@bvarner-ebi - happy for you to close this and go through your processes :) thanks

@shawntanzk
Copy link
Contributor Author

We're requesting outside collaborators to request PR reviewers instead of automatically assigning them.

btws @bvarner-ebi sorry for assigning directly, I figured that was the way to request PR reviewers. Do let me know how I should go about it in the future. Personally I dont really mind who reviews my PRs, but I tend to find that if I don't assign a reviewer, the PR sits there and nothing happens. Is there a system to pick up PR reviews in CL if I dont really mind who reviews my PR?

Currently I am following CL documentation to assign a reviewer https://obophenotype.github.io/cell-ontology/contributing/#pull-request-guidelines quoting the docs:

Make sure pull requests have someone assigned to review them and remind them once in a while. Do not let them go dormant

Happy to follow whatever procedure is preferred by the community, but would like it documented cause different ontologies have different preferences in workflow and all :)

PS on a seperate note, I notice that @gouttegd was to be generic assignee for tickets in the documentation lol - I think this was written up when he was sheparding CL tickets, might consider a change?

Copy link
Contributor

Choose a reason for hiding this comment

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

why do you have changes in the merged_import.owl?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

can revert them - happen the term to be obsolete has a uberon term in there, and so obsoletion in protege removes/change all axioms, one (or more) of which comes from import. I have opened another ticket for uberon already, so better way would probably be to let this be and make sure uberon side changes, gets imported into cl, before release - of course with the 2 weeks notification, this has got to be coordinated and all, so will leave that to you all. For now I will revert changes in merged_import

@ghost
Copy link

ghost commented Aug 18, 2023

This PR involves obsoleting a CL class.

Will close without merging for now (acknowledged by assignee) with following action items:

  • When time permits, editor will review ticket, if necessary find literature support to justify change
  • notify slack #cl-editors of planned obsoletion
  • if no objection after at least 2 weeks, proceed with merge
  • next CL release is planned for next week, so if there are no objections change will be reflected in September release at the earliest
  • work with @anitacaron to incorporate synonym QC in CL checks (see Add synonym QC check to CL #2111)

@shawntanzk, thanks again for spotting these potential changes to improve CL.

@ghost ghost closed this Aug 18, 2023
@anitacaron anitacaron deleted the issue-2101 branch May 20, 2024 15:34
This pull request was closed.
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.

CL:0009077 & CL:0002294 are duplicates?
3 participants