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

Fix heatmap bug in representing deletions. #183

Merged
merged 6 commits into from
May 7, 2024

Conversation

jstone-dev
Copy link
Collaborator

@jstone-dev jstone-dev commented Apr 29, 2024

This fixes issue #139, in which it was reported that deletions appear in the top row of the heatmap, normally reserved for substitutions by Ala.

In addition:

  • Some refactoring, TypeScript migration, and JSDoc creation has also been done.
  • The heatmap's y-axis tick mark labels have been made more readable.

@PlushZ provided a useful example in score set tmp:540c9d47-9e0c-4c56-839c-3b1249a957df. To test the bug fix using this example, you may change the ownership of that score set in a development instance of MaveDB.

The heatmap for this score set should now look like this:
heatmap-tmp-540c9d47-9e0c-4c56-839c-3b1249a957df

The heatmap for score set urn:mavedb:00000001-a-1 is a good check against regressions, and it should look like this:
heatmap-urn-mavedb-00000001-a-1

export function parseSimpleProVariant(variant: string): SimpleProteinVariation | null {
const match = variant.match(proVariantRegex)
if (!match) {
// console.log(`WARNING: Unrecognized pro variant: ${variant}`)
Copy link
Contributor

Choose a reason for hiding this comment

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

should probably be un-commented or deleted

// .tickFormat(n => String.fromCharCode('A'.charCodeAt(0) + n - 1))
.tickFormat((n) => {
// Get the row's amino acid code or variation symbol.
const label = HEATMAP_ROWS[HEATMAP_ROWS.length - 1 - n]
Copy link
Contributor

Choose a reason for hiding this comment

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

repeating this logic to reverse the HEATMAP_ROWS gets very confusing to the reader.

I think there are a few options here.

  1. reverse heatmap_rows
  2. use a d3 scalePoint as in this notebook to combine the flipping and translation of letter to axis position: https://observablehq.com/@mbostock/phases-of-the-moon
  3. to eliminate the backwards iteration below, use a data structure for each heatmap_rows entry with style and label attributes that have defaults, so we have entries like {'symbol': '-', 'style': 'mave-heatmap-y-axis-tick-label-lg'} and {'symbol': '*', 'label': '\uff0a'}

* @return The one-character code representing the same amino acid or change, or null if the input was not a supported
* amino acid or change.
*/
export function singleLetterAminoAcidOrHgvsCode(aaCodeOrChange: string) {
Copy link
Contributor

Choose a reason for hiding this comment

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

let's do explicit return types for ts in this project

@ashsny ashsny changed the base branch from main to release-2024.1.0 May 2, 2024 23:03
@ashsny ashsny merged commit ac38786 into release-2024.1.0 May 7, 2024
@jstone-dev jstone-dev deleted the jstone-uw/heatmap-deletion-fix branch May 15, 2024 20:06
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.

2 participants