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

Update STR page allele size histogram to show colors based on genotype quality or other values #1649

Open
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

bw2
Copy link
Contributor

@bw2 bw2 commented Oct 31, 2024

remaining TODOs:

  • add legend
  • add color to genotype plot?

Copy link
Contributor

@rileyhgrant rileyhgrant left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me!

Really nice work.

I left a few comments in line, one about typing, two very minor things, and one "nice one" for reuse of the your sorting hook.

@phildarnowsky-broad
Copy link
Contributor

@rileyhgrant this latest should address all your feedback

@rileyhgrant rileyhgrant self-requested a review February 26, 2025 15:48
Copy link
Contributor

@rileyhgrant rileyhgrant left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for addressing those so promptly, I just left one minor comment about two !s being (possibly) mistakenly dropped.

After double checking that, LGTM!

Comment on lines 47 to 50
repeatUnitsByClassification.pathogenic &&
repeatUnitsByClassification.pathogenic.length === 1 &&
repeatUnitsByClassification.benign &&
repeatUnitsByClassification.unknown
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like the ! may have been mistakenly dropped from these last two conditions:

before:

(repeatUnitsByClassification as any).pathogenic &&
(repeatUnitsByClassification as any).pathogenic.length === 1 &&
!(repeatUnitsByClassification as any).benign &&
!(repeatUnitsByClassification as any).unknown

after:

repeatUnitsByClassification.pathogenic &&
repeatUnitsByClassification.pathogenic.length === 1 &&
repeatUnitsByClassification.benign &&
repeatUnitsByClassification.unknown

Just wanted to flag this in case it was unintentional!

Major changes here are:

* Instead of a single `reference_region`, STRs now have a list of `reference_regions` with a single one designated the `main_reference_region`
* Allele size distributions and genotype distributions were previously represented with an attempt to represent multidimensional data with a number of nested structs, which was serviceable when there were only one or two dimensions we might want to filter on, but was getting increasingly convoluted. Since this new data expands the number of dimensions further, rather than build on the former schema and confuse things more, these distributions are now represented with a flattened list of structs each of which represents a single subset of the distribution.
Key changes:

* Allele size distribution plot can now show, by use of stacked bars, breakdown of each bucket by population, quality, or sex. This also involved replacing some of our custom logic with calls to the visx family of libraries.
* More options for y-scaling of allele size distribution plot.
* Assorted refactoring, type specifying, and similar cleanup
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