statgen / pheweb

A tool to build a website to browse hundreds or thousands of GWAS.
MIT License
158 stars 65 forks source link

Add GWAS catalog to PheWeb #112

Closed abought closed 5 years ago

abought commented 5 years ago

Ticket: #17

Purpose

Add a GWAS catalog plot to PheWeb, based on an upcoming LZ.js release

screen shot 2018-09-25 at 3 05 44 am

TODO

Recommendations for discussion

Known issues

Summary of changes

How to test this

abought commented 5 years ago

I've added back the field extraction mechanism. It does impose a namespacing restriction- this is still a change to PheWeb core behavior, but it makes it easier to use the same data source with all the other layers that already work that way.

Per the tooltip issue, I've traced this to pointer-events:none, the tooltip workaround. We're also seeing it with the exac link on genes tooltips in a modern pheweb. Will delegate that one to Peter as this was his workaround and he has more context. http://pheweb.sph.umich.edu:5003/region/110.12/2:113925484-114325484

abought commented 5 years ago

Also fixed the phewas scatter plot tooltips. @pjvandehaar , when would be a good time to meet and parcel out the remaining tasks needed to move this forward?

abecasis commented 5 years ago

Great to see this happen. It would be great for PheWeb to serve as showcase of what locuszoom can do!

Sent from my iPhone

On Sep 25, 2018, at 3:24 AM, Andy Boughton notifications@github.com wrote:

Ticket: #17

Purpose

Add a GWAS catalog plot to PheWeb, based on an upcoming LZ.js release

TODO

Recommendations for discussion

Add test data that has a sample region with SNPs in the catalog The catalog track only labels SNPs that are in high LD. Consider adding the "Make LD reference" link back to Pheweb assoc scatter plot tooltips, to encourage exploration. Known issues

The PheWAS coloring mechanism has changed; update this so phewas plots are colored correctly GWAS catalog rug track tooltips are not properly clickable (can't click the GWAS catalog link; investigate pheweb overrides) Since this updates LZ.js to the newest version, generally review app for breakages The layout is slightly off kilter; revise and generally tweak positioning (some "spacer" panels were removed in the layout reconciliation, as their purpose was unclear) Summary of changes

Replace in-repo LZ.js with a version from a CDN DRY layouts to use override mechanism Region plots now use "association_catalog" instead of "standard_association" for their base plot layout (and appropriate child layouts, with modifications) Change association source: remove override of a deprecated function, in favor of explicit namespaced field requests (revisit this) How to test this

Version 0.9.0 has not yet been released yet; we'll need to do that before PheWeb can be released! I've been testing with local static files, representing the tip of the LZ.js develop branch (0a0df23) You can view, comment on, or merge this pull request online at:

https://github.com/statgen/pheweb/pull/112

Commit Summary

Use CDN version for LZjs, and remove locally stored version. Updates for LZ.js 0.9 Reconcile duplicate region plot layout with LZjs core overrides File Changes

M pheweb/conf_utils.py (1) M pheweb/serve/server.py (2) M pheweb/serve/static/region.js (610) M pheweb/serve/static/variant.js (29) D pheweb/serve/static/vendor/locuszoom-0.5.6-locuszoom.css (792) D pheweb/serve/static/vendor/locuszoom-0.5.6.app.js (8892) D pheweb/serve/static/vendor/locuszoom-0.5.6.vendor.min.js (6) M pheweb/serve/templates/gene.html (8) M pheweb/serve/templates/region.html (7) M pheweb/serve/templates/variant.html (9) Patch Links:

https://github.com/statgen/pheweb/pull/112.patch https://github.com/statgen/pheweb/pull/112.diff — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or mute the thread.

abought commented 5 years ago

No word from Peter as yet. In the interests of meeting ASHG deadlines, and recognizing that it's already been ~10 days, I'm going to put go ahead and make most of the layout and behavior changes originally marked "for discussion", plus a few other tweaks.

Some of the things I held off on could introduce possible regressions, but we can discuss the details after the fact. Will update more tomorrow.

abought commented 5 years ago

Tooltip functionality changed to support more granular hyperlinks (not just "click on dot to go to phewas").

Still need to identify test data with SNPs in the GWAS catalog; Peter would be more efficient at adding this. This concludes my current queue of work for PheWeb integration until we've looked over the direction and design plan.

screen shot 2018-10-05 at 12 38 04 am
sgagliano commented 5 years ago

Thanks Andy for all your work on this feature. Peter, when are you able to address this PR?

abought commented 5 years ago

I've pushed (most of) the changes from discussion branch https://github.com/statgen/pheweb/compare/8cdb7...pull-112-head#diff-007e08bf75bfe53b85b14034803e369c . These are intended as a companion to statgen/locuszoom#150.

I'll merge the LZ.js PR tonight barring any further requests.

abought commented 5 years ago

A note: tests are passing locally (after incorporating the test_all fix- thanks!)

They are failing on travis even after restarting the build, but the error message is opaque. Peter, have you seen that failure log before?

pjvandehaar commented 5 years ago

I’ll try a different gencode url later.

abought commented 5 years ago

Gencode url? I might be missing context here- what sort of different data do you need?

(Would any of the pending LZ data source changes be of use?)

abought commented 5 years ago

LocusZoom 0.9 was officially released in late December, containing several of the items requested/used by PheWeb.

What is the status of this PR as a possible candidate for merge? Does it need updating?

Given the historical timeline, I'll hold off on making revisions until I hear back, but help is available on that basis.

pjvandehaar commented 5 years ago

This is now on master.