Trait association search #267
Conversation
* Added a web component that allows retrieving studies by genus, species, study type, traits, DOI, PMID, and author. * HTML file added incorrectly. * Added an explanation of what the element does into the html example. * For some reason the result rendering didn't get added to the earlier commit. Fixed. * Added a web component that allows retrieving studies by genus, species, study type, traits, DOI, PMID, and author. * HTML file added incorrectly. * Added an explanation of what the element does into the html example. * For some reason the result rendering didn't get added to the earlier commit. Fixed. * Updated trait association search to fit new version of graphql. Still needs work. * Removed console.log(data) from example html. * Added genus limiting parameter to trait association search. Modified docs. Rebuilt docs. * Fixing issues with query. * Minor change to docs and html element. * Added a web component that allows retrieving studies by genus, species, study type, traits, DOI, PMID, and author. * HTML file added incorrectly. * Added an explanation of what the element does into the html example. * For some reason the result rendering didn't get added to the earlier commit. Fixed. * Updated trait association search to fit new version of graphql. Still needs work. * Removed console.log(data) from example html. * Added genus limiting parameter to trait association search. Modified docs. Rebuilt docs. * Fixing issues with query. * Minor change to docs and html element. * Fixed linting issues. * Added a web component that allows retrieving studies by genus, species, study type, traits, DOI, PMID, and author. * HTML file added incorrectly. * Added an explanation of what the element does into the html example. * For some reason the result rendering didn't get added to the earlier commit. Fixed. * Updated trait association search to fit new version of graphql. Still needs work. * Removed console.log(data) from example html. * Added genus limiting parameter to trait association search. Modified docs. Rebuilt docs. * Fixing issues with query. * Minor change to docs and html element. * Fixed linting issues. * Rebuilt docs. * Apply prettier formatting changes and check with eslint * Restoring files back to main because of accidental changes. * Changed form classes. * Changed the way results are displayed to match that of the other search elements. * Removed depreciated, commented out code. * Apply prettier formatting changes and check with eslint * Update to support new filter parameters in graphql query. Will make things much faster over the previous version. * Removed unnecessary console log and comments. * Apply prettier formatting changes and check with eslint * Added limiting search form by species. * Apply prettier formatting changes and check with eslint --------- Co-authored-by: That-Thing <sen@onee.ch> Co-authored-by: That-Thing <That-Thing@users.noreply.github.com>
Specifically, variables were renamed and comments were updated to make the component consistent with itself and the existing codebase. A couple minor bugs were fixed as well.
These revisions are basically a subset of the previously committed revision to the trait association search.
The property was ported from the trait association search.
Specifically, the method is in the paginated search mixin and returns the given value, if defined, or the value of the specified querystring parameter, if defined, otherwise it returns the empty string. The use case this supports is preferring optional component properties over querystring parameters but it can be used for values that aren't properties as well.
Specifically, using an empty string for the genus and species properties of the gene search and trait association search components will now pre-set their genus and species selectors to the default "all" value.
…perties. If you provided an invalid value for the genus or species property of the gene search or trait association search component then the respective selectors would be set to the default "any" value. However, when the form was submitted the invalid value would be sent to the search function instead of the default value. This has been fixed.
There was a problem hiding this comment.
The Genus/species filter doesn't work. This is expected since there is no genus or species filter in search-traits.ts in graphql-server:main. That functionality is implemented in graphql-server:5.1.0.4-model-changes. So, running against MiniMine, which has Phaseolus vulgaris and Vigna unguiculata GWAS and QTL Studies, you get results when genus/species is set to Glycine max.
I think we figured those selectors would be frozen on Glycine max on the Soybase site, and all the GWAS and QTL studies in SoyMine are for Glycine max (well, a couple are for soja, but who cares).
But, technically, the UI doesn't work, and I'd say it should therefore not be merged into main until graphql-server:5.1.0.4-model-changes is merged into main. Soybase can use this particular branch in the meantime.
|
Also, the results don't link anything as far as I can tell. |
I believe the plan is to merge this and tag a release so Soybase can use the packaged version on NPM via the theme. They're planning to use the
What do you mean? Like hyperlinks to studies? That's a site-specific feature that should be coded when the component is added to a site, just like the gene search component. Speaking of which, we should probably add scripts to the theme for that sooner than later. |
|
Understood. I'll hold my ground on this: if a As for the trait result linkouts, right, I'm looking at the code under |
|
OK, I don't want to block soybase getting this into production so I'll merge when @That-Thing has had a chance to look at it. In general I don't think the status of a specific mine or the graphql-server should have any impact on merging/releasing web components. The |
Well, I care about stuff working, and the way I test that is in the use case that I care about: web-components + graphql-server on MiniMine for testing against deployment to jekyll-legumeinfo with graphql-server on LegumeMine. If a web-component doesn't work in that environment then I'm not inclined to approve a merge. @That-Thing 's scenario is different, so he'll presumably approve it for his use case. I'll continue testing stuff for use on the LIS Jekyll site and approve/disapprove accordingly. I get it about the linkouts, in that case I'll need to test them on my dev jekyll-legumeinfo site like with theme stuff. |
| const results = | ||
| data.traits.results.map(({name, ...trait}) => { | ||
| const type = trait.qtlStudy ? 'QTL' : 'GWAS'; | ||
| const study = trait.qtlStudy || trait.gwas; |
There was a problem hiding this comment.
It seems that when leaving the trait field blank, the element ends up getting a result which does not have a qtlStudy or gwas object, which causes a TypeError (TypeError: study is null).
Adding a way to handle such cases would probably be a good idea.
There was a problem hiding this comment.
Interesting. I encountered this error while working on this PR but didn't dig in too deeply: legumeinfo/graphql-server#104. Let's see what @sammyjava thinks.
There was a problem hiding this comment.
There are no traits that lack both QTLStudy and GWAS references in GlycineMine:
glycinemine-5103=> select * from trait where qtlstudyid is null and gwasid is null;
description | id | name | primaryidentifier | qtlstudyid | gwasid | datasetid | class
-------------+----+------+-------------------+------------+--------+-----------+-------
(0 rows)
A traits query that requests both qtlStudy and gwas WILL return a null gwas on non-null qtlStudy and a null qtlStudy on a non-null gwas, nothing surprising there. Is that causing an issue? That is expected behavior. I don't see how to get a Trait without either a GWAS or a QTLStudy unless I explicitly do not request those from the query (and then they aren't in the JSON, either).
This against MiniMine since GlycineMine has 924 trait records:
{
"data": {
"traits": {
"results": [
{
"identifier": "Cerinza_x_G24404.qtl.Blair_Iriarte_2006:Days_to_flowering",
"gwas": null,
"qtlStudy": {
"identifier": "Cerinza_x_G24404.qtl.Blair_Iriarte_2006"
}
},
{
"identifier": "DOR364_x_BAT477.qtl.Blair_Galeano_2012:Days_to_flowering",
"gwas": null,
"qtlStudy": {
"identifier": "DOR364_x_BAT477.qtl.Blair_Galeano_2012"
}
},
{
"identifier": "mixed.gwas.Raggi_Caproni_2019:Days_to_flowering",
"gwas": {
"identifier": "mixed.gwas.Raggi_Caproni_2019"
},
"qtlStudy": null
},
{
"identifier": "Cerinza_x_G24404.qtl.Blair_Iriarte_2006:Days_to_maturity",
"gwas": null,
"qtlStudy": {
"identifier": "Cerinza_x_G24404.qtl.Blair_Iriarte_2006"
}
},
{
"identifier": "DOR364_x_BAT477.qtl.Blair_Galeano_2012:Days_to_maturity",
"gwas": null,
"qtlStudy": {
"identifier": "DOR364_x_BAT477.qtl.Blair_Galeano_2012"
}
},
{
"identifier": "mixed.gwas.Myers_Wallace_2019:Flower_color",
"gwas": {
"identifier": "mixed.gwas.Myers_Wallace_2019"
},
"qtlStudy": null
},
{
"identifier": "mixed.gwas.Myers_Wallace_2019:Immature_pod_color__blue_to_yellow",
"gwas": {
"identifier": "mixed.gwas.Myers_Wallace_2019"
},
"qtlStudy": null
},
{
"identifier": "mixed.gwas.Myers_Wallace_2019:Immature_pod_color__green_to_red",
"gwas": {
"identifier": "mixed.gwas.Myers_Wallace_2019"
},
"qtlStudy": null
},
{
"identifier": "mixed.gwas.Myers_Wallace_2019:Immature_pod_color__lightness",
"gwas": {
"identifier": "mixed.gwas.Myers_Wallace_2019"
},
"qtlStudy": null
},
{
"identifier": "Cerinza_x_G24404.qtl.Blair_Iriarte_2006:Plant_height",
"gwas": null,
"qtlStudy": {
"identifier": "Cerinza_x_G24404.qtl.Blair_Iriarte_2006"
}
},
{
"identifier": "Cerinza_x_G24404.qtl.Blair_Iriarte_2006:Plant_width",
"gwas": null,
"qtlStudy": {
"identifier": "Cerinza_x_G24404.qtl.Blair_Iriarte_2006"
}
},
{
"identifier": "ZN016_x_Zhijiang282.gwas.Xu_Wu_2017:Pod_length",
"gwas": {
"identifier": "ZN016_x_Zhijiang282.gwas.Xu_Wu_2017"
},
"qtlStudy": null
},
{
"identifier": "ZN016_x_Zhijiang282.qtl.Xu_Wu_2017:Pod_length",
"gwas": null,
"qtlStudy": {
"identifier": "ZN016_x_Zhijiang282.qtl.Xu_Wu_2017"
}
},
{
"identifier": "mixed.gwas.Myers_Wallace_2019:Pod_phenol",
"gwas": {
"identifier": "mixed.gwas.Myers_Wallace_2019"
},
"qtlStudy": null
},
{
"identifier": "Cerinza_x_G24404.qtl.Blair_Iriarte_2006:Pods_per_plant",
"gwas": null,
"qtlStudy": {
"identifier": "Cerinza_x_G24404.qtl.Blair_Iriarte_2006"
}
},
{
"identifier": "Cerinza_x_G24404.qtl.Blair_Iriarte_2006:Seed_weight",
"gwas": null,
"qtlStudy": {
"identifier": "Cerinza_x_G24404.qtl.Blair_Iriarte_2006"
}
},
{
"identifier": "DOR364_x_BAT477.qtl.Blair_Galeano_2012:Seed_weight",
"gwas": null,
"qtlStudy": {
"identifier": "DOR364_x_BAT477.qtl.Blair_Galeano_2012"
}
},
{
"identifier": "Cerinza_x_G24404.qtl.Blair_Iriarte_2006:Seeds_per_plant",
"gwas": null,
"qtlStudy": {
"identifier": "Cerinza_x_G24404.qtl.Blair_Iriarte_2006"
}
},
{
"identifier": "mixed.gwas.Jain_Poromarto_2019:Soybean_cyst_nematode",
"gwas": {
"identifier": "mixed.gwas.Jain_Poromarto_2019"
},
"qtlStudy": null
},
{
"identifier": "Cerinza_x_G24404.qtl.Blair_Iriarte_2006:Yield",
"gwas": null,
"qtlStudy": {
"identifier": "Cerinza_x_G24404.qtl.Blair_Iriarte_2006"
}
},
{
"identifier": "DOR364_x_BAT477.qtl.Blair_Galeano_2012:Yield",
"gwas": null,
"qtlStudy": {
"identifier": "DOR364_x_BAT477.qtl.Blair_Galeano_2012"
}
},
{
"identifier": "DOR364_x_BAT477.qtl.Blair_Galeano_2012:Yield_per_day",
"gwas": null,
"qtlStudy": {
"identifier": "DOR364_x_BAT477.qtl.Blair_Galeano_2012"
}
}
]
}
}
}There was a problem hiding this comment.
Needless to say, if you can give me the query that is misbehaved I can look into it. I don't see a problem so far.
There was a problem hiding this comment.
Query:
query TraitQuery($pageSize: Int, $page: Int, $name: String, $studyType: String, $publicationId: String, $author: String) {
traits(pageSize: $pageSize, page: $page, name: $name, studyType: $studyType, publicationId: $publicationId, author: $author) {
results {
name
qtlStudy {
identifier
synopsis
description
genotypes
organism {
genus
species
}
dataSet {
publication {
firstAuthor
pubMedId
doi
}
}
}
gwas {
identifier
synopsis
description
genotypes
organism {
genus
species
}
dataSet {
publication {
firstAuthor
pubMedId
doi
}
}
}
}
pageInfo {
currentPage
pageSize
numResults
pageCount
hasPreviousPage
hasNextPage
}
}
}
Variables:
{
"author": "",
"genus": "Glycine",
"name": "",
"page": 1,
"pageSize": 10,
"publicationId": "",
"species": "Max",
"studyType": "GWAS"
}
There was a problem hiding this comment.
So apparently this was an issue with intermine, not graphql-server; see legumeinfo/graphql-server#104 (comment). Regardless, the question now is do we want to update the code in this PR to be resilient to such missing data?
Currently all the components are dumb. They just take the data given to them and draw it on the screen. If data is missing then nothing is drawn. I think we should leave that as is. However, it's probably worth updating the code that shims the data from GraphQL into what the component needs so that it doesn't break if something is missing that shouldn't be. Here's a diff that does this for the trait association search:
diff --git a/dev/lis-trait-association-search-element.html b/dev/lis-trait-association-search-element.html
index 38556e7..401dba0 100644
--- a/dev/lis-trait-association-search-element.html
+++ b/dev/lis-trait-association-search-element.html
@@ -168,8 +168,8 @@
// flatten results
const results =
data.traits.results.map(({name, ...trait}) => {
- const type = trait.qtlStudy ? 'QTL' : 'GWAS';
- const study = trait.qtlStudy || trait.gwas;
+ const type = trait.qtlStudy ? 'QTL' : trait.gwas ? 'GWAS' : undefined;
+ const study = trait.qtlStudy || trait.gwas || {};
const {identifier, synopsis, description, genotypes} = study;
return {
identifier,Thoughts?
There was a problem hiding this comment.
I don't think I'd have discovered the error in those two LIS Datastore collections if we hadn't hit this error here. The idea with the mines is that the data are "perfect". The mines break pretty easily if not. So I fixed those two GWAS collections and added validation so I don't load data with that problem again. (OH, I better do that for QTL study collections as well, I bet there are a few in there!)
So I'm saying there is actually an advantage to graphql-server breaking when the mine data is errant. We don't really have the users to send us emails when stuff is broken and we don't use the mines much ourselves.
There was a problem hiding this comment.
(AHA I already had that validation check for QTL collections, I just didn't have it for GWAS collections! That explains things.)
There was a problem hiding this comment.
So I'm saying there is actually an advantage to graphql-server breaking when the mine data is errant. We don't really have the users to send us emails when stuff is broken and we don't use the mines much ourselves.
I'm fine with that approach, in which case I think the component is functioning as intended and we can merge for Soybase's sake.
There was a problem hiding this comment.
Yup, the GlycineMine update just finished (@That-Thing ) and I checked on those four errant traits and we're all good. Merge for Soybase.
This PR builds on @That-Thing's initial implementation of the trait association search web component. It also includes some changes to the gene search web component that are strictly limited to features and bug fixes introduced in the trait association search component that should be ported to the gene search component.