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.

Sign up for GitHub

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

Jump to bottom

Add slim coverage check running on local (branch) copy CL #2182

Merged
merged 17 commits into from
Oct 24, 2023
Merged

Add slim coverage check running on local (branch) copy CL #2182

merged 17 commits into from
Oct 24, 2023

Conversation

ubyndr
Copy link
Contributor

@ubyndr ubyndr commented Oct 12, 2023 •
edited
Loading

Resolves #2177

We decided not to follow the GitHub action route in this issue. Editors should use the following commands to run coverage reports via the Makefile in the ontology directory.;

  • for specific upper_slim report:
sh run.sh make reports/general_cell_types_upper_slim_report.csv
sh run.sh make reports/blood_and_immune_upper_slim_report.csv
sh run.sh make reports/eye_upper_slim_report.csv
  • for all upper_slim reports:
sh run.sh make slim_coverage

cc @aleixpuigb @bvarner-ebi

ubyndr and others added 9 commits October 12, 2023 17:07
@ubyndr
Refactored generic_coverage.py to work on local copy of cl-base.owl
e5038b8
Merge branch 'master' into 2177-add-slim-coverage-check-as-github-act…
b3a84b3 
…ion-running-on-local-branch-copy-cl
@ubyndr
Type hinting fix
37257c4
@ubyndr
Merge branch '2177-add-slim-coverage-check-as-github-action-running-o…
df7f256 
…n-local-branch-copy-cl' of https://github.com/obophenotype/cell-ontology into 2177-add-slim-coverage-check-as-github-action-running-on-local-branch-copy-cl
@ubyndr
Added a new argument to COVERAGECMD, and changed TERM_general, CL:000…
75f84da 
…0548 to CL:0000000
@ubyndr
Added a new argument to COVERAGECMD, and changed TERM_general, CL:000…
0abbc57 
…0548 to CL:0000000
Merge branch 'master' into 2177-add-slim-coverage-check-as-github-act…
e49bddc 
…ion-running-on-local-branch-copy-cl
Merge branch 'master' into 2177-add-slim-coverage-check-as-github-act…
ff6183c 
…ion-running-on-local-branch-copy-cl
@ubyndr
Updated generic_coverage.py
d5cd193
@ghost ghost mentioned this pull request Oct 17, 2023
Closed
anitacaron
anitacaron reviewed Oct 17, 2023
View reviewed changes
requirements.txt Outdated Show resolved Hide resolved
src/scripts/requirements.txt Outdated Show resolved Hide resolved
@ubyndr ubyndr marked this pull request as ready for review October 23, 2023 09:57
@ubyndr ubyndr requested review from aleixpuigb and a user October 23, 2023 09:57
aleixpuigb
aleixpuigb previously approved these changes Oct 23, 2023
View reviewed changes
Copy link
Collaborator

@aleixpuigb aleixpuigb left a comment •
edited
Loading

Choose a reason for hiding this comment

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

The tests performed have been successful, with a running time of 20 minutes.

@anitacaron anitacaron changed the title Add slim coverage check as GitHub Action running on local (branch) copy CL Add slim coverage check running on local (branch) copy CL Oct 23, 2023
anitacaron
anitacaron reviewed Oct 23, 2023
View reviewed changes
src/scripts/generic_coverage.py
@@ -98,7 +127,6 @@ def get_scope_query(scope_term: str) -> str:
WHERE
{{
?scope_member rdfs:subClassOf|BFO:0000050|RO:0002100 {_scope} .
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this getting the same result as before? Query an OWL file is not direct as in Ubergraph. Need to add the existential part.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

relation-graph command should help with these, shouldn't it? Did I misinterpreted the command's function?

Copy link
Contributor

Choose a reason for hiding this comment

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

Need to double-check.

aleixpuigb
aleixpuigb requested changes Oct 23, 2023
View reviewed changes
Copy link
Collaborator

@aleixpuigb aleixpuigb left a comment •
edited
Loading

Choose a reason for hiding this comment

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

I have tested the eye slim with eye UBERON:0000970 this time and the coverage includes UBERON terms which should not. Moreover, it doesn't cover many cell types that it used to. Are the object properties overlaps or 'has soma location' (subproperty of overlaps) included?
The running time was 13:30 minutes.

@ubyndr
Copy link
Contributor Author

ubyndr commented Oct 23, 2023

I have tested the eye slim with eye UBERON:0000970 this time and the coverage includes UBERON terms which should not. Moreover, it doesn't cover many cell types that it used to. Are the object properties overlaps or 'has soma location' (subproperty of overlaps) included? The running time was 13:30 minutes.

I'm aware of this as it was reported by @bvarner-ebi. I'm trying to debug the script.

anitacaron
anitacaron reviewed Oct 23, 2023
View reviewed changes
src/scripts/generic_coverage.py
@@ -124,7 +152,6 @@ def get_superclass_value_query(term_iri_list: List[str], _scope: str) -> str:
{{
?term rdfs:subClassOf ?super. ?super rdfs:label ?label.
?super rdfs:subClassOf|BFO:0000050 {_scope}.
?super rdfs:isDefinedBy <http://purl.obolibrary.org/obo/cl.owl> .
Copy link
Contributor

Choose a reason for hiding this comment

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

Should change it to filter:

FILTER (STRSTARTS(STR(?super), "http://purl.obolibrary.org/obo/CL_"))

@ubyndr ubyndr requested a review from aleixpuigb October 24, 2023 09:05
ghost
ghost approved these changes Oct 24, 2023
View reviewed changes
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Report successfully generated

aleixpuigb
aleixpuigb approved these changes Oct 24, 2023
View reviewed changes
Copy link
Collaborator

@aleixpuigb aleixpuigb left a comment

Choose a reason for hiding this comment

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

The report has been generated with the expected coverage and uncovered terms.

@aleixpuigb aleixpuigb merged commit c253377 into master Oct 24, 2023
1 check passed
@aleixpuigb aleixpuigb deleted the 2177-add-slim-coverage-check-as-github-action-running-on-local-branch-copy-cl branch October 24, 2023 12:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@anitacaron anitacaron anitacaron left review comments

@aleixpuigb aleixpuigb aleixpuigb approved these changes

Assignees

@ubyndr ubyndr

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add slim coverage check as GitHub Action running on local (branch) copy CL
3 participants
@ubyndr @anitacaron @aleixpuigb