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

use cancer type in CIViC results #28

Merged

Conversation

HomoPolyethylen
Copy link
Contributor

PR Checklist

  • This comment contains a description of changes (with reason)
  • Referenced issue is linked
  • If you've fixed a bug or added code that should be tested, add tests!
  • Documentation in docs is updated
  • CHANGELOG.rst is updated

Description of changes

  • added functionality to specify cancer type to civic queries. (new CL param -d / --cancer-doid)

Technical details

  • user specifies a cancer type
  • querynator filters evidence items in results:
    • include evidences from same cancer type
    • include evidences from higher-level cancer type (i.e. when specifying "Cholangiocarcinoma", also include all evidences for "Bile duct adenocarcinoma")
    • include level A evidences for other cancer type as level C evidence for this cancer type (as described in AMP/ASCO/CAT guidelines)

Additional context

CGI and CIViC use different ontologies (CGI: OncoTree, CIViC: Disease-Ontology. Right now they need to be specified separately, passing the burden to map ontologies to the user.
This should be attended to in the future. Let user specify one cancer type, querynator maps to OncoTree / DO, queries, ...

@HomoPolyethylen HomoPolyethylen self-assigned this May 6, 2024
Copy link

@famosab famosab left a comment

Choose a reason for hiding this comment

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

As far as I can tell it looks good :)

docs/usage.rst Show resolved Hide resolved
querynator/helper_functions/ontology.py Outdated Show resolved Hide resolved
querynator/query_api/civic_api.py Outdated Show resolved Hide resolved
@HomoPolyethylen HomoPolyethylen marked this pull request as ready for review May 7, 2024 11:26
@HomoPolyethylen HomoPolyethylen linked an issue May 8, 2024 that may be closed by this pull request
1 task
Copy link
Collaborator

@mapo9 mapo9 left a comment

Choose a reason for hiding this comment

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

looks great so far!
I only have 2 minor comments

docs/usage.rst Outdated Show resolved Hide resolved
querynator/__main__.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@mapo9 mapo9 left a comment

Choose a reason for hiding this comment

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

LGTM! 🚀

@mapo9 mapo9 self-requested a review June 20, 2024 13:27
Copy link
Collaborator

@mapo9 mapo9 left a comment

Choose a reason for hiding this comment

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

did you check why the tests are failing?

@famosab
Copy link

famosab commented Jun 20, 2024

New changes look good to me.

Pytest seems to run into some kind of numpy error:
ValueError: numpy.dtype size changed, may indicate binary incompatibility. Expected 96 from C header, got 88 from PyObject

StackOverflow has a solution I think: https://stackoverflow.com/questions/78634235/numpy-dtype-size-changed-may-indicate-binary-incompatibility-expected-96-from

@famosab
Copy link

famosab commented Jun 20, 2024

You'll need to fix the version of numpy to 1.26.4 for example (probably in the requirements file).

@HomoPolyethylen HomoPolyethylen merged commit e6e097b into qbic-pipelines:dev Jun 23, 2024
9 checks passed
@HomoPolyethylen HomoPolyethylen deleted the HomoPolyethylen/issue27 branch June 23, 2024 12:56
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.

use cancer type in civic
3 participants