statgen / locuszoom

A Javascript/d3 embeddable plugin for interactively visualizing statistical genetic data from customizable sources.
https://statgen.github.io/locuszoom/
MIT License
154 stars 29 forks source link

[rfr] Update PheWAS plot for new API #113

Closed abought closed 7 years ago

abought commented 7 years ago

Purpose

Expand the PheWAS plot example to work with the new API endpoint data sources, and to not rely on hardcoded tick mark labels.

Sample endpoint URL: http://portaldev.sph.umich.edu/api_internal_dev/v1/statistic/phewas/?filter=variant eq '10:114758349_C/T'&format=objects&build=GRCh37

This represents a new user-level feature to support T2D portal work, with a target external release date of Oct 1.

Summary of changes

PheWAS-related

Internal hooks/ functionality

screen shot 2017-09-25 at 12 08 37 pm screen shot 2017-09-22 at 12 21 23 pm

TODO/ Notes

Known issues:

Frencil commented 7 years ago

Tonight I've updated and solidified the tick generation pathways on #112. Here are some relevant parts of the diff:

welchr commented 7 years ago

In some cases it looks like it's labeling a point with the trait label, and in other cases it isn't. Example:

image

If you merge in my PR, the first example on the left sidebar will trigger the behavior, but the third link down will not.

abought commented 7 years ago

In some cases it looks like it's labeling a point with the trait label, and in other cases it isn't.

Yeah, this appears to be a regression. Labels are subject to filters (p>=20 as default threshold), but it seems to be labeling only the first.

abought commented 7 years ago

For the preview, I'm going to declare this branch complete as is and begin my own final round of QA + documentation work. Worlds of thanks to @pjvandehaar , @welchr , and @Frencil for feedback and helpful improvements.

If anyone has feedback before the end of the day let me know. Otherwise plan is to update app.js and merge this by COB today.

This PR contains testing for a lot of the new stuff, but I'll aim to fill in remaining gaps in test coverage later in the week, as schedule permits. My sincere apologies for the messy commit history, but the rebase looked even gnarlier.

welchr commented 7 years ago

Thanks to all of you for the hard work on this. It looks great. :) I don't think anyone here is a real commit history purist (other than maybe Peter...)

When I see it merge into master (or wait until you msg me the OK?) I'll send an email to the Broad folks.

Though I do have one question on that. Once it's merged into master, could you forward gh-pages up to master so I can point them to http://statgen.github.io/locuszoom/examples/phewas_scatter.html (instead of the quick dummy portaldev page I was using before)?

abought commented 7 years ago

Found one issue during my QA and fixed. I also did light regression testing of our other plot types.

Below are the release notes I put together, and I'll merge after my 2pm meeting. (AFAIK, only ChrisC has push access for the npm package, but that's in no way urgent)

LocusZoom 0.6.0 - Release notes

User-facing features

Internal improvements

Bugfixes

Possible breaking changes

welchr commented 7 years ago

I'm not actually sure if the Broad uses our npm package. My impression was they used code from our repository directly. We should probably update it regardless just in case anyone else may be using it.

@Frencil can you add Andy to the npm package so he can modify as needed?