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

Tangletree genotype colouring #1785

Merged
merged 6 commits into from
Jun 10, 2024

Conversation

jameshadfield
Copy link
Member

See commit messages for details.

Closes #1773

  • Checks pass
  • If making user-facing changes, add a message in CHANGELOG.md summarizing the changes in this PR

This property will be used in upcoming work to show genotype colourings
on both trees
@nextstrain-bot nextstrain-bot temporarily deployed to auspice-james-tangletre-tqlnzr June 10, 2024 07:48 Inactive
@jameshadfield jameshadfield requested a review from a team June 10, 2024 07:57
@jameshadfield jameshadfield force-pushed the james/tangletree-genotype-colouring branch from e120c86 to de5ab6e Compare June 10, 2024 09:19
@nextstrain-bot nextstrain-bot temporarily deployed to auspice-james-tangletre-tqlnzr June 10, 2024 09:19 Inactive
This particular method of loading a second tree is (I believe) rarely
used and rarely developed -- see both the TODOs in the surrounding code
and the (dev-only) redux immutability errors fixed in this commit.
Still, we should support it!
In preparation for showing genotype colorings across multiple trees

We don't (yet) consider root-sequences which are defined via
sidecar files (i.e. not inline in the main JSON)

We could improve memory efficiency if we compared the nuc/aa sequences
for equality and used references to the main tree's root-sequence data,
but as we're here only considering inlined root-sequences (i.e. small
ones!) this implementation is simpler.
@jameshadfield jameshadfield force-pushed the james/tangletree-genotype-colouring branch from de5ab6e to 8324f8a Compare June 10, 2024 09:45
@nextstrain-bot nextstrain-bot temporarily deployed to auspice-james-tangletre-tqlnzr June 10, 2024 09:45 Inactive
src/util/setGenotype.js Outdated Show resolved Hide resolved
Includes some (overly!) strict conditionals. This functionality is
particularly useful when comparing trees using the same (or very
similar) sequences / sets of sequences to aid in understanding the
different tree structures.

Closes #1773
Oh how TypeScript would have helped!

This bug was found while debugging the ability to show genotype
colorings on both trees (added in the parent commit) where the
visibility of the second tree was `undefined` upon loading, leading to
`createVisibleLegendValues` not considering the second tree and thus
removing legend items which were only observed in the second tree.

Bug introduced ~4 years ago via 391bca4
although its effect was minor (and probably never noticed?)
@nextstrain-bot nextstrain-bot temporarily deployed to auspice-james-tangletre-tqlnzr June 10, 2024 21:39 Inactive
@jameshadfield jameshadfield merged commit c11938d into master Jun 10, 2024
19 of 20 checks passed
@jameshadfield jameshadfield deleted the james/tangletree-genotype-colouring branch June 10, 2024 21:40
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.

[tanglegrams] genotype is only computed for LHS tree
3 participants