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.

Sign up for GitHub

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

Color tree by measurements #1924

Merged
merged 35 commits into from
Jan 21, 2025
Merged

Color tree by measurements #1924

Changes from 1 commit
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
d90c503
MeasurementsControlState: Remove unnecessary trailing commas
joverlee521 Dec 18, 2024
6c2e53b
measurements: Fix bug for numeric legend values in ordering
joverlee521 Dec 12, 2024
15dabdf
Add color tree by measurements feature
joverlee521 Dec 10, 2024
14a5650
ADD_EXTRA_METADATA: only use non-continuous colorings as filter traits
joverlee521 Dec 19, 2024
1391b9b
applyMeasurementsColorBy: Use multiple anchors for color scale
joverlee521 Dec 11, 2024
acf11a4
measurements: Refactor getting matching filters into separate function
joverlee521 Dec 12, 2024
a4b4d9c
applyMeasurementsColorBy: Only include measurements that pass filters
joverlee521 Dec 13, 2024
69853e2
measurements: Save color grouping value as a separate state
joverlee521 Dec 18, 2024
8b2773e
applyMeasurementsColorBy: remove old measurements coloring data
joverlee521 Dec 20, 2024
74eac4a
applyMeasurementsColorBy: batch dispatch actions
joverlee521 Dec 20, 2024
c0b7e91
Refactor `applyMeasurementsColorBy` into small functions
joverlee521 Dec 19, 2024
e1c74a4
Update measurements coloring when changing collection
joverlee521 Dec 24, 2024
9a5a183
Update measurements coloring when changing group by
joverlee521 Dec 24, 2024
3c35ed5
Update measurement coloring when updating measurements filters
joverlee521 Dec 21, 2024
c2db2bb
Update type `ColoringInfo`
joverlee521 Dec 24, 2024
17d411c
Refactor `addMeasurementsColorData` into smaller function
joverlee521 Dec 24, 2024
ade2c20
Support measurements coloring URL param `c=m-<grouping_value>`
joverlee521 Dec 24, 2024
440cddd
Add crosshair in measurements panel y-axis for coloring
joverlee521 Dec 30, 2024
ff28517
measurements: Allow click on grouping when current colorby is different
joverlee521 Dec 31, 2024
795efc7
Remove extra whitespace
joverlee521 Jan 2, 2025
9a4c328
tree: Add crosshair on tip matching measurements color grouping
joverlee521 Jan 2, 2025
5d51e9c
tree: re-draw measurements crosshair when modifying SVG
joverlee521 Jan 3, 2025
0dbc4e3
tree: update measurements crosshair when changing colorBy
joverlee521 Jan 3, 2025
fb3eba0
narratives: Ensure tree coloring is calculated for measurements colors
joverlee521 Jan 3, 2025
a2cfe54
narratives: Ensure old measurements coloring data is removed
joverlee521 Jan 3, 2025
bb44c2a
modifySVGInStages: remove crosshair in step 1
joverlee521 Jan 6, 2025
82e6f6d
Add comments for measurementColoringPrefix
joverlee521 Jan 7, 2025
9f6b032
Add tooltip to hint at color by measurements feature
joverlee521 Jan 7, 2025
27cdbb0
Click on measurements grouping to toggle back to previous coloring
joverlee521 Jan 8, 2025
e52f48d
Update type ColoringInfo: make `scale` optional
joverlee521 Jan 9, 2025
972ed91
Add `_hasMeasurementColor` attr and coloring for measurements coloring
joverlee521 Jan 9, 2025
e586a6c
Filter to colored tips when applying measurements coloring
joverlee521 Jan 9, 2025
921161c
measurements: Use `String` instead of `toString`
joverlee521 Jan 21, 2025
aead1cf
removeNodeAttrs: drop unnecessary conditional
joverlee521 Jan 21, 2025
daccd78
Update changelog
joverlee521 Jan 21, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/components/measurements/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,8 @@ const filterMeasurements = (
const MeasurementsPlot = ({height, width, showLegend, setPanelTitle}) => {
// Use `lodash.isEqual` to deep compare object states to prevent unnecessary re-renderings of the component
const { treeStrainVisibility, treeStrainColors } = useSelector((state: RootState) => treeStrainPropertySelector(state), isEqual);
const legendValues = useSelector((state: RootState) => state.controls.colorScale.legendValues, isEqual);
// Convert legendValues to string to ensure that subsequent attribute matches work as intended
const legendValues = useSelector((state: RootState) => state.controls.colorScale.legendValues.map((v): string => v.toString()), isEqual);
Copy link
Member

Choose a reason for hiding this comment

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

A few minor points - don't change them if you don't agree with them:

Since our attributes are all strings due to the use of Object.keys(),
this did not match the legend values when they were numeric and resulted
in the ordering of mean and raw values to be arbitrary. This has
been a bug all along, but I didn't notice it until I was working on the
coloring by measurements feature that focused on continuous numeric
legend values.

This commit converts all legend values to strings in the Redux
selector so that ordering works across all color scales.

  • toString will raise an exception on certain values (null, undefined etc) whereas String will not, and the latter can be succinctly written as .map(String). The legendValues are any[] so these values may occur? I'm surprised tsc didn't complain.
  • Not being super familiar with measurements terminology, I didn't understand "attributes" in the commit message and had to dive into the code. I think it's node attributes / node values, right? Where is the Object.keys you mention - I followed the code a little bit but didn't find it.

Copy link
Contributor Author

@joverlee521 joverlee521 Jan 21, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Ah good point about toString raising exceptions, I'll update to String. (Edit: updated in 921161c.)

Not being super familiar with measurements terminology, I didn't understand "attributes" in the commit message and had to dive into the code. I think it's node attributes / node values, right?

These are referring to the coloring attributes, so yes these correspond to the node attributes / node values.

Where is the Object.keys you mention - I followed the code a little bit but didn't find it.

These are referring to where the node attributes get compared to the legend values for ordering. These are in measurementsD3.js, within jitterRawMeansByColorBy and drawMeansForColorBy.

const colorings = useSelector((state: RootState) => state.metadata.colorings);
const colorBy = useSelector((state: RootState) => state.controls.colorBy);
joverlee521 marked this conversation as resolved.
Show resolved Hide resolved
const groupBy = useSelector((state: RootState) => state.controls.measurementsGroupBy);
Expand Down