-
Notifications
You must be signed in to change notification settings - Fork 2
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
feat: add search filters for gene (clinically actionable, druggable g… #496
base: dev
Are you sure you want to change the base?
Conversation
…enome, drug resistance)
hmmm @mcannon068nw I'm debating removing the fields I added for |
FWIW I would agree that it can be dangerous to introduce a second way to do a thing that you can already do. IMO the graphql interface needs to just work and be efficient, and user friendliness/convenience can be implemented in consumer packages like rdgidb/dgipy. |
I approved this in principle, but a few other considerations/stuff I noticed:
query {
genes(druggableGenome:true, name:"BRAF"){
edges {
node {
conceptId
clinicallyActionable
}
}
}
} the stuff you added is correct but it's creating a conflict with the way the option(:name, type: String, description: 'Left anchored string search on gene symbol') do |scope, value|
scope.where('genes.name ILIKE ?', "#{value.upcase}%")
# ^ added genes table name here
end |
It might be worth it to rethink this then. I think when I was envisioning this issue, I was trying to think of more ways to introduce potentially useful filtering at different layers of the query, not necessarily just the first level gene search. As an example, I believe I was thinking about a case like this: Where the filter is just done as an additional parameter and only returns results that are clinically actionable, or a kinase, or part of the druggable genome. Would love some additional thoughts on this @jsstevenson @katiestahl |
^^ yeah that totally makes sense to me. We might want to double-check that this isn't a search that you could just do on |
…enome, drug resistance)
close #479
I went ahead and also added these as fields to access for genes in
gene_type.rb
. Let me know whether or not we want to keep that @mcannon068nw . Otherwise, the core part that addresses the requirement isgenes.rb