EBISPOT / goci

GWAS Catalog Ontology and Curation Infrastructure
Apache License 2.0
26 stars 19 forks source link

Refocusing UI to allow exclusion of background traits #165

Closed ljwh2 closed 3 years ago

ljwh2 commented 3 years ago

in road map due April

ljwh2 commented 3 years ago

https://docs.google.com/document/d/1agaBcPxuCws0pKnuZh_PJylk5TO84FG9YGgpxIXzZpY/edit?usp=sharing

eks-ebi commented 3 years ago

Testing 2021-06-14:

eks-ebi commented 3 years ago

Oddly, for the example study above with the incorrectly annotated associations (http://snoopy.ebi.ac.uk:9580/gwas/studies/GCST90013538), when I click on the individual variant links, the resulting Variant pages do not contain any studies or associations (e.g. http://snoopy.ebi.ac.uk:9580/gwas/variants/rs6017006)... So maybe there is something else going on with this study

ala-ebi commented 3 years ago

@eks-ebi good catch! data in DB and in Solr Search server is correct, it's UI thing, I'm investigating. Update: Variant page is broken cause it relies on Slim Solr which is outdated on DEV 1. It's back up now (using prod Slim Solr). As for the association's traits, issue was found in DB.

ala-ebi commented 3 years ago

Investigated the issue with the study being split and associations are not. The issue is that this Study was not provided in the Excel sheet that contains the Studies to split. So it must've been split by a curator, but only for Studies, not for associations (background traits for associations is a new addition of this Splitting Issue).

ljwh2 commented 3 years ago

@eks-ebi The study that has split studies but not associations was a submission, I wonder if that had some effect? Do you have any other examples of submissions with a background trait?

ljwh2 commented 3 years ago

Overall functionality looks great!

I agree scientifically it would be better to exclude background traits by default if possible (unchecked box) BUT this requires us to define behaviour of traits which are only background (note - an example of this is Trypanosoma cruzi seropositivity). We can discuss, but on balance it may be better to leave as is.

I noticed an issue for traits that have both child terms and background traits. Unchecking background traits seems to exclude child traits as well. Unchecking and rechecking child traits adds them back in. Example: hypertension EFO:0000537 755 associations, 91 studies. Uncheck background traits: 394 associations, 33 studies. Uncheck child traits as well: 394 associations, 33 studies. Recheck child traits: 515 associations, 58 studies.

Another example is schizophrenia EFO:0000692, behaviour is the same.

ala-ebi commented 3 years ago

Cheers, well spotted! I have deployed a fix.

ljwh2 commented 3 years ago

@ala-ebi Re-tested checking and unchecking child & background boxes, works as expected now.

We discussed and decided it would be better to have the background traits box unticked by default.

ala-ebi commented 3 years ago

'Apply efo traits to associaitons' to associations now takes into account background traits as well, they can be seen later on in the association page image.png

eks-ebi commented 3 years ago

Tested UI again today: I've looked through all the different pages (Trait, Study etc.) and everything looks correct. Ticking/unticking the bg trait box working fine.

Not sure if anything else specific needs testing?

eks-ebi commented 3 years ago

Testing the Curation interface:

The 'Apply efo traits to associations' button works well:

  1. I added a bg trait to a study that already had a main trait --> the button left the main trait as it was and added bg trait to all associations. Working as expected.
  2. I then changed the bg trait to something different and pressed the button again --> the old bg trait was removed from all assocs and replaced with the new one. Working as expected.
eks-ebi commented 3 years ago

@ala-ebi let me know if there's anything else that needs testing

eks-ebi commented 3 years ago

We discussed adding a tooltip for the background trait field in the UI.

Proposed text: "A trait that is not directly analysed in the GWAS, but is shared by all study participants as a common characteristic." @ljwh2 Does this look ok?

ljwh2 commented 3 years ago

Yes I'm happy with that @eks-ebi

ala-ebi commented 3 years ago

filled background traits in 'All studies v1.0.3' downloads

eks-ebi commented 3 years ago

@ala-ebi I've checked 'All studies v1.0.3' download and all of the background traits seem to be displayed correctly.

eks-ebi commented 3 years ago

I've also checked the tooltip mentioned above and it looks good on the Study page. But I think it would be good to also display the same tooltip in other places where the term "background trait" appears:

  1. In the column header in the Associations and Studies tables: Screenshot 2021-07-21 at 09.22.25.png Screenshot 2021-07-21 at 09.24.21.png

  2. On the Trait page next to the background traits checkbox Screenshot 2021-07-21 at 09.25.44.png

@ljwh2 Would you agree?

ala-ebi commented 3 years ago

done

eks-ebi commented 3 years ago

FAQs now updated to explain background traits, committed to documentation branch: https://github.com/EBISPOT/gwas-ui/blob/documentation/goci-interfaces/goci-ui/src/main/docs/faq-content.adoc