dgidb / dgidb-v5

Providing interactions between drugs and genes sourced from a variety of publications and knowledgebases
https://dgidb.org
MIT License
14 stars 2 forks source link

feat: add search filters for gene (clinically actionable, druggable g… #496

Open katiestahl opened 3 months ago

katiestahl commented 3 months ago

…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 is genes.rb

katiestahl commented 3 months ago

hmmm @mcannon068nw I'm debating removing the fields I added for gene_type since they're kinda unnecessary/redundant since a user can just look at the categories to synthesize that information if desired. On one hand, it's nice in the response for quick use and easy to find with a quick glance. But it also is a bit redundant since it's simplified info that's already available in the response... Thoughts?

jsstevenson commented 3 months ago

hmmm @mcannon068nw I'm debating removing the fields I added for gene_type since they're kinda unnecessary/redundant since a user can just look at the categories to synthesize that information if desired. On one hand, it's nice in the response for quick use and easy to find with a quick glance. But it also is a bit redundant since it's simplified info that's already available in the response... Thoughts?

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.

jsstevenson commented 3 months ago

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 name arg is defined, to fix you need to change that arg definition to this:

  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
mcannon068nw commented 2 months ago

hmmm @mcannon068nw I'm debating removing the fields I added for gene_type since they're kinda unnecessary/redundant since a user can just look at the categories to synthesize that information if desired. On one hand, it's nice in the response for quick use and easy to find with a quick glance. But it also is a bit redundant since it's simplified info that's already available in the response... Thoughts?

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.

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:

Screenshot 2024-06-04 at 9 58 43 AM

What if our researcher was hypothetically only interested in interactions for genes of a particular category, such as clinically actionable or KINASE? They could definitely write some post-processing code to filter based on whats returned in the gene categories block, but wouldn't it also be useful to just have something like this:

Screenshot 2024-06-04 at 9 59 10 AM

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

jsstevenson commented 2 months ago

^^ 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 interactions but if not, then it totally makes sense