Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Regression line only considers visible nodes #1484

Merged
merged 2 commits into from
Mar 16, 2022
Merged

Conversation

jameshadfield
Copy link
Member

See #1483 for context. A couple of before (left) vs after (right) screenshots:

image
image

This commit updates the PhyloTree logic to call the recalculate &
redraw functions each time the visibility changes. Note that the
regression calculation does not yet take account of the visible nodes
so there should be no functionality changes in this commit.
Changes the regression calculations to only consider visible nodes.

Closes #1483

Note this has the (rare but strange) UI situation where we have zero
visible tips, and thus the regression line isn't drawn, but the
sidebar toggle still indicates that the regression is "on".
@nextstrain-bot nextstrain-bot temporarily deployed to auspice-fix-regression-dcxebqb March 16, 2022 00:23 Inactive
@trvrb
Copy link
Member

trvrb commented Mar 16, 2022

Awesome! Thanks for the quick work here James. I noticed this originally when I was attempting to explain to someone that you should still less clock correlation with a dataset that spans a narrower time window. With PR, this works as expected, where you can go from R^2 of 0.82 for the full ~2 year dataset to an R^2 of 0.33 for dataset spanning ~6 months.

Really cool to be able to drag the date slider and watch the regression line update live in response.

Everything seems to be working well in my testing. Nice that you catch the empty dataset edge case.

@jameshadfield jameshadfield merged commit b990c6c into master Mar 16, 2022
@jameshadfield jameshadfield deleted the fix/regression branch March 16, 2022 02:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants