Fix download empty tree #1591
Fix download empty tree #1591
Conversation
In cases where the user has filtered the tree, the `idxOfFilteredRoot` is the root of the selected tips instead of `idxOfInViewRootNode`. By using `idxOfFilteredRoot` as the root of the tree, this now acts the same way as the manual work-around of clicking "Zoom to selected" then downloading the tree. However, this does _not_ fix the case where the filter may have created multiple subtrees. If there are multiple subtrees without a most recent common ancestor, this function will still produce an empty Newick tree.
Raise an error when `treeToNewick` creates an empty tree is created because this is undesired behavior. Created a custom error class for this so that we could include details in the warning notification for the user.
nextstrain-bot
temporarily deployed
to
auspice-fix-download-em-gc2rs3
There was a problem hiding this comment.
Thanks Jover! I agree that erroring in most situations like this is preferable -- newick trees can't represent the states that auspice can be in! We could probably work this out on download-modal rendering & not even allow downloading to be attempted, but the approach in this PR is just fine.
As an aside, the logic around zooming / inView nodes / visible nodes is getting pretty complex. In part this is because a lot of those features have been added over time and we haven't rethought the underlying architecture. Zooming is well overdue a rethink in auspice so hopefully we can address this then!
Definitely! Would love to revisit the xy zoom idea. |
Description of proposed changes
Fixes the empty tree downloads when the tree is filtered.
This does not support the case of downloading multiple subtrees. I'm going to punt that for now and just added a note to the users that we do not support this feature. We can open another issue if this feature is ever requested.
I've made the
treeToNewickfunction raise an error if it ever creates an empty tree since this seems like generally undesired behavior. I can drop this change if people think otherwise.Related issue(s)
Resolves #1267
Testing