-
Notifications
You must be signed in to change notification settings - Fork 35
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
Dot plot #1383
base: main
Are you sure you want to change the base?
Dot plot #1383
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you maybe post a conf I could try out? I want to be sure the multi-selection feature works but I can't seem to get it. I have
{
component: 'featureList',
props: {
enableMultiSelect: true,
},
x: 9,
y: 0,
w: 3,
h: 2,
},
@@ -35,7 +35,7 @@ export function FeatureListSubscriber(props) { | |||
variablesLabelOverride, | |||
theme, | |||
title: titleOverride, | |||
enableMultiSelect = false, | |||
enableMultiSelect = true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure this is working?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, this is a pretty big default change, no?
/** | ||
* Get expression data for the cells | ||
* in the selected cell sets. | ||
* @param {object} expressionMatrix |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the given gene selection, and also it doesn't have the rows and cols I think.
mergedCellSets, cellSetSelection, cellIndices, | ||
); | ||
geneSelection.forEach((featureName, featureI) => { | ||
const featureKey = uuidv4(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this for duplicated gene names?
// Get the max characters in an axis label for autsizing the bottom margin. | ||
const maxCharactersForGroup = data.reduce((acc, val) => { | ||
// eslint-disable-next-line no-param-reassign | ||
acc = acc === undefined || val.group.length > acc ? val.group.length : acc; | ||
return acc; | ||
}, 0); | ||
const maxCharactersForFeature = data.reduce((acc, val) => { | ||
// eslint-disable-next-line no-param-reassign | ||
acc = acc === undefined || val.feature.length > acc ? val.feature.length : acc; | ||
return acc; | ||
}, 0); | ||
// Use a square-root term because the angle of the labels is 45 degrees (see below) | ||
// so the perpendicular distance to the bottom of the labels is proportional to the | ||
// square root of the length of the labels along the imaginary hypotenuse. | ||
// 30 is an estimate of the pixel size of a given character and seems to work well. | ||
const autoMarginVertical = marginBottom | ||
|| 30 + Math.sqrt(maxCharactersForFeature / 2) * 30; | ||
const autoMarginHorizontal = marginRight | ||
|| 30 + Math.sqrt(maxCharactersForGroup / 2) * 30; | ||
|
||
const plotWidth = clamp(width - autoMarginHorizontal - 120, 10, Infinity); | ||
const plotHeight = clamp(height - autoMarginVertical, 10, Infinity); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This trick is used elsewhere, no? Should it refactored into its own hook or function?
Fixes #754
Fixes #1040
TODO
posThreshold
Checklist
vitessce-python
andvitessce-r
if this is a release PR