From f920bba3210628101812448591f1e1ee4c2009cd Mon Sep 17 00:00:00 2001 From: Peyton Lee Date: Fri, 22 Mar 2024 15:45:46 -0700 Subject: [PATCH 01/16] feat: Added utility function for converting to lowercase + underscore characters --- src/colorizer/utils/data_utils.ts | 6 +++++- tests/data_utils.test.ts | 32 +++++++++++++++++++++++++++++++ 2 files changed, 37 insertions(+), 1 deletion(-) create mode 100644 tests/data_utils.test.ts diff --git a/src/colorizer/utils/data_utils.ts b/src/colorizer/utils/data_utils.ts index 1e922a0a5..618400625 100644 --- a/src/colorizer/utils/data_utils.ts +++ b/src/colorizer/utils/data_utils.ts @@ -1,6 +1,6 @@ import { MAX_FEATURE_CATEGORIES } from "../../constants"; import { ColorRampData } from "../colors/color_ramps"; -import { FeatureThreshold, isThresholdCategorical, isThresholdNumeric,ThresholdType } from "../types"; +import { FeatureThreshold, isThresholdCategorical, isThresholdNumeric, ThresholdType } from "../types"; import ColorRamp from "../ColorRamp"; import Dataset, { FeatureType } from "../Dataset"; @@ -121,3 +121,7 @@ export function getInRangeLUT(dataset: Dataset, thresholds: FeatureThreshold[]): } return inRangeIds; } + +export function getKeyFromName(name: string): string { + return name.toLowerCase().replaceAll(/[^a-z0-9_]/g, "_"); +} diff --git a/tests/data_utils.test.ts b/tests/data_utils.test.ts new file mode 100644 index 000000000..277801c42 --- /dev/null +++ b/tests/data_utils.test.ts @@ -0,0 +1,32 @@ +import { describe, expect, it } from "vitest"; + +import { getKeyFromName } from "../src/colorizer/utils/data_utils"; + +describe("data_utils", () => { + describe("getKeyFromName", () => { + it("handles empty strings", () => { + expect(getKeyFromName("")).toBe(""); + }); + + it("allows alphanumeric and underscore characters", () => { + expect(getKeyFromName("a")).toBe("a"); + expect(getKeyFromName("_")).toBe("_"); + expect(getKeyFromName("az09")).toBe("az09"); + expect(getKeyFromName("a_b_c")).toBe("a_b_c"); + expect(getKeyFromName("abc_123")).toBe("abc_123"); + }); + + it("sets alphabetic characters to lowercase", () => { + expect(getKeyFromName("A")).toBe("a"); + expect(getKeyFromName("Z")).toBe("z"); + expect(getKeyFromName("ABCDEFG")).toBe("abcdefg"); + }); + + it("replaces non-alphanumeric characters with underscores", () => { + expect(getKeyFromName("+")).toBe("_"); + expect(getKeyFromName("...")).toBe("___"); + expect(getKeyFromName("Some Name (a)")).toBe("some_name__a_"); + expect(getKeyFromName("&Another Name%")).toBe("_another_name_"); + }); + }); +}); From f63cbbee40e04b00dce730ebb98bdf64dcf0e8c8 Mon Sep 17 00:00:00 2001 From: Peyton Lee Date: Fri, 22 Mar 2024 16:09:36 -0700 Subject: [PATCH 02/16] feat: Parsed feature keys from dataset manifest --- src/colorizer/Dataset.ts | 6 ++++++ src/colorizer/utils/dataset_utils.ts | 6 +++++- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/src/colorizer/Dataset.ts b/src/colorizer/Dataset.ts index ff1b96d18..e1ba1c1a7 100644 --- a/src/colorizer/Dataset.ts +++ b/src/colorizer/Dataset.ts @@ -2,6 +2,7 @@ import { RGBAFormat, RGBAIntegerFormat, Texture, Vector2 } from "three"; import { MAX_FEATURE_CATEGORIES } from "../constants"; import { FeatureArrayType, FeatureDataType } from "./types"; +import { getKeyFromName } from "./utils/data_utils"; import { AnyManifestFile, ManifestFile, ManifestFileMetadata, updateManifestVersion } from "./utils/dataset_utils"; import * as urlUtils from "./utils/url_utils"; @@ -18,6 +19,8 @@ export enum FeatureType { } export type FeatureData = { + name: string; + key: string; data: Float32Array; tex: Texture; min: number; @@ -57,6 +60,7 @@ export default class Dataset { private arrayLoader: IArrayLoader; // Use map to enforce ordering + /** Maps, in order, from feature names to feature data. */ private features: Map; private outlierFile?: string; @@ -144,6 +148,8 @@ export default class Dataset { return [ name, { + name, + key: metadata.key || getKeyFromName(name), tex: source.getTexture(FeatureDataType.F32), data: source.getBuffer(FeatureDataType.F32), min: source.getMin(), diff --git a/src/colorizer/utils/dataset_utils.ts b/src/colorizer/utils/dataset_utils.ts index 8826bacfa..1243a52d6 100644 --- a/src/colorizer/utils/dataset_utils.ts +++ b/src/colorizer/utils/dataset_utils.ts @@ -1,5 +1,6 @@ // Defines types for working with dataset manifests, and methods for // updating manifests from one version to another. +import { getKeyFromName } from "./data_utils"; import { Spread } from "./type_utils"; // eslint-disable-next-line @typescript-eslint/naming-convention @@ -57,6 +58,7 @@ type ManifestFileV0_0_0 = { // eslint-disable-next-line @typescript-eslint/naming-convention type ManifestFileV1_0_0 = Omit & { features: { + key?: string; name: string; data: string; units?: string; @@ -104,6 +106,7 @@ export const updateManifestVersion = (manifest: AnyManifestFile): ManifestFile = const featureMetadata = manifest.featureMetadata?.[featureName]; features.push({ name: featureName, + key: getKeyFromName(featureName), data: featurePath, units: featureMetadata?.units || undefined, type: featureMetadata?.type || undefined, @@ -111,10 +114,11 @@ export const updateManifestVersion = (manifest: AnyManifestFile): ManifestFile = }); } - return { + manifest = { ...manifest, features, }; } + return manifest; }; From a8c070ce8946670d39ef3bb52522e1aebc9e8aaa Mon Sep 17 00:00:00 2001 From: Peyton Lee Date: Fri, 22 Mar 2024 16:45:23 -0700 Subject: [PATCH 03/16] refactor: Use `featureKey` instead of `featureName` --- src/Viewer.tsx | 114 +++++++++++++------------ src/colorizer/ColorizeCanvas.ts | 18 ++-- src/colorizer/Dataset.ts | 72 ++++++++++------ src/colorizer/types.ts | 2 +- src/colorizer/utils/data_utils.ts | 22 ++--- src/colorizer/utils/url_utils.ts | 22 +++-- src/components/PlotWrapper.tsx | 6 +- src/components/Tabs/PlotTab.tsx | 4 +- src/components/Tabs/ScatterPlotTab.tsx | 18 ++-- tests/Dataset.test.ts | 20 +++-- 10 files changed, 168 insertions(+), 130 deletions(-) diff --git a/src/Viewer.tsx b/src/Viewer.tsx index 4f0d7bdb2..ae525f0cb 100644 --- a/src/Viewer.tsx +++ b/src/Viewer.tsx @@ -73,7 +73,7 @@ function Viewer(): ReactElement { const [dataset, setDataset] = useState(null); const [datasetKey, setDatasetKey] = useState(""); - const [featureName, setFeatureName] = useState(""); + const [featureKey, setFeatureKey] = useState(""); const [selectedTrack, setSelectedTrack] = useState(null); const [currentFrame, setCurrentFrame] = useState(0); const [selectedBackdropKey, setSelectedBackdropKey] = useState(null); @@ -109,10 +109,10 @@ function Viewer(): ReactElement { // Check if the current feature is being thresholded on, and if that threshold // has changed. If so, snap the current min + max color ramp values so they match the new // threshold values. - const featureData = dataset?.getFeatureData(featureName); + const featureData = dataset?.getFeatureData(featureKey); if (featureData) { - const oldThreshold = featureThresholds.find(thresholdMatchFinder(featureName, featureData.units)); - const newThreshold = newThresholds.find(thresholdMatchFinder(featureName, featureData.units)); + const oldThreshold = featureThresholds.find(thresholdMatchFinder(featureKey, featureData.units)); + const newThreshold = newThresholds.find(thresholdMatchFinder(featureKey, featureData.units)); if (newThreshold && oldThreshold && isThresholdNumeric(newThreshold) && isThresholdNumeric(oldThreshold)) { if (newThreshold.min !== oldThreshold.min || newThreshold.max !== oldThreshold.max) { @@ -123,7 +123,7 @@ function Viewer(): ReactElement { } _setFeatureThresholds(newThresholds); }, - [featureName, dataset, featureThresholds] + [featureKey, dataset, featureThresholds] ); /** A look-up-table from object ID to whether it is in range (=1) or not (=0) */ const inRangeLUT = useMemo(() => { @@ -192,14 +192,14 @@ function Viewer(): ReactElement { } // check if current selected feature range matches the default feature range; if so, don't provide // a range parameter.. - const featureData = dataset.getFeatureData(featureName); + const featureData = dataset.getFeatureData(featureKey); if (featureData) { if (featureData.min === colorRampMin && featureData.max === colorRampMax) { return undefined; } } return [colorRampMin, colorRampMax]; - }, [colorRampMin, colorRampMax, featureName, dataset]); + }, [colorRampMin, colorRampMax, featureKey, dataset]); /** * Get a URL query string representing the current collection, dataset, feature, track, @@ -211,7 +211,7 @@ function Viewer(): ReactElement { const state: Partial = { collection: collectionParam, dataset: datasetParam, - feature: featureName, + featureKey: featureKey, track: selectedTrack?.trackId, // Ignore time=0 to reduce clutter time: currentFrame !== 0 ? currentFrame : undefined, @@ -228,7 +228,7 @@ function Viewer(): ReactElement { }, [ getDatasetAndCollectionParam, getRangeParam, - featureName, + featureKey, selectedTrack, currentFrame, featureThresholds, @@ -277,27 +277,27 @@ function Viewer(): ReactElement { } setFindTrackInput("" + trackId); }, - [canv, dataset, featureName, currentFrame] + [canv, dataset, featureKey, currentFrame] ); /** * Attempts to replace the current feature with a new feature from a dataset. - * If the feature cannot be loaded, returns the old feature name and does nothing. + * If the feature cannot be loaded, returns the old feature key and does nothing. * @param newDataset the dataset to pull feature data from. - * @param newFeatureName the name of the new feature to select. - * @returns the new feature name if it was successfully found and loaded. Otherwise, returns the old feature name. + * @param newFeatureKey the key of the new feature to select. + * @returns the new feature key if it was successfully found and loaded. Otherwise, returns the old feature key. */ const replaceFeature = useCallback( - (featureDataset: Dataset, newFeatureName: string): string => { - if (!featureDataset?.hasFeature(newFeatureName)) { - console.warn("Dataset does not have feature '" + newFeatureName + "'."); - return featureName; + (featureDataset: Dataset, newFeatureKey: string): string => { + if (!featureDataset?.hasFeatureKey(newFeatureKey)) { + console.warn("Dataset does not have feature '" + newFeatureKey + "'."); + return featureKey; } - setFeatureName(newFeatureName); - canv.setFeature(newFeatureName); - return newFeatureName; + setFeatureKey(newFeatureKey); + canv.setFeatureKey(newFeatureKey); + return newFeatureKey; }, - [canv, featureName] + [canv, featureKey] ); /** @@ -309,11 +309,11 @@ function Viewer(): ReactElement { * (Does nothing if the viewer is configured to keep the range between datasets.) */ const resetColorRampRangeToDefaults = useCallback( - (featureDataset: Dataset, featureName: string): void => { - const featureData = featureDataset.getFeatureData(featureName); + (featureDataset: Dataset, featureKey: string): void => { + const featureData = featureDataset.getFeatureData(featureKey); if (!config.keepRangeBetweenDatasets && featureData) { // Use min/max from threshold if there is a matching one, otherwise use feature min/max - const threshold = featureThresholds.find(thresholdMatchFinder(featureName, featureData.units)); + const threshold = featureThresholds.find(thresholdMatchFinder(featureKey, featureData.units)); if (threshold && isThresholdNumeric(threshold)) { setColorRampMin(threshold.min); setColorRampMax(threshold.max); @@ -352,16 +352,17 @@ function Viewer(): ReactElement { setDatasetKey(newDatasetKey); // Only change the feature if there's no equivalent in the new dataset - let newFeatureName = featureName; - if (!newDataset.hasFeature(newFeatureName)) { - newFeatureName = newDataset.featureNames[0]; + let newFeatureKey = featureKey; + if (!newDataset.hasFeatureKey(newFeatureKey)) { + // No equivalent, so default to first feature + newFeatureKey = newDataset.featureKeys[0]; } - replaceFeature(newDataset, newFeatureName); - resetColorRampRangeToDefaults(newDataset, newFeatureName); - setFeatureName(newFeatureName); + replaceFeature(newDataset, newFeatureKey); + resetColorRampRangeToDefaults(newDataset, newFeatureKey); + setFeatureKey(newFeatureKey); await canv.setDataset(newDataset); - canv.setFeature(newFeatureName); + canv.setFeatureKey(newFeatureKey); // Clamp frame to new range const newFrame = Math.min(currentFrame, canv.getTotalFrames() - 1); @@ -378,7 +379,7 @@ function Viewer(): ReactElement { }, [ dataset, - featureName, + featureKey, canv, currentFrame, getUrlParams, @@ -493,10 +494,10 @@ function Viewer(): ReactElement { setFeatureThresholds(initialUrlParams.thresholds); } } - let newFeatureName = featureName; + let newFeatureKey = featureKey; if (initialUrlParams.feature && dataset) { // Load feature (if unset, do nothing because replaceDataset already loads a default) - newFeatureName = replaceFeature(dataset, initialUrlParams.feature); + newFeatureKey = replaceFeature(dataset, initialUrlParams.feature); } // Range, track, and time setting must be done after the dataset and feature is set. if (initialUrlParams.range) { @@ -504,7 +505,7 @@ function Viewer(): ReactElement { setColorRampMax(initialUrlParams.range[1]); } else { // Load default range from dataset for the current feature - dataset && resetColorRampRangeToDefaults(dataset, newFeatureName); + dataset && resetColorRampRangeToDefaults(dataset, newFeatureKey); } if (initialUrlParams.track && initialUrlParams.track >= 0) { @@ -575,11 +576,11 @@ function Viewer(): ReactElement { const getFeatureValue = useCallback( (id: number): string => { - if (!featureName || !dataset) { + if (!featureKey || !dataset) { return ""; } // Look up feature value from id - const featureData = dataset.getFeatureData(featureName); + const featureData = dataset.getFeatureData(featureKey); // ?? is a nullish coalescing operator; it checks for null + undefined values // (safe for falsy values like 0 or NaN, which are valid feature values) const featureValue = featureData?.data[id] ?? -1; @@ -587,7 +588,7 @@ function Viewer(): ReactElement { // Check if int, otherwise return float return numberToStringDecimal(featureValue, 3) + unitsLabel; }, - [featureName, dataset] + [featureKey, dataset] ); // SCRUBBING CONTROLS //////////////////////////////////////////////////// @@ -667,8 +668,8 @@ function Viewer(): ReactElement { return []; } // Add units to the dataset feature names if present - return dataset.featureNames.map((name) => { - return { key: name, label: dataset.getFeatureNameWithUnits(name) }; + return dataset.featureKeys.map((key) => { + return { key, label: dataset.getFeatureNameWithUnits(key) }; }); }, [dataset]); @@ -678,11 +679,11 @@ function Viewer(): ReactElement { // Show min + max marks on the color ramp slider if a feature is selected and // is currently being thresholded/filtered on. const getColorMapSliderMarks = (): undefined | number[] => { - const featureData = dataset?.getFeatureData(featureName); + const featureData = dataset?.getFeatureData(featureKey); if (!featureData || featureThresholds.length === 0) { return undefined; } - const threshold = featureThresholds.find(thresholdMatchFinder(featureName, featureData.units)); + const threshold = featureThresholds.find(thresholdMatchFinder(featureKey, featureData.units)); if (!threshold || !isThresholdNumeric(threshold)) { return undefined; } @@ -692,7 +693,7 @@ function Viewer(): ReactElement { let hoveredFeatureValue = ""; if (lastHoveredId !== null && dataset) { const featureVal = getFeatureValue(lastHoveredId); - const categories = dataset.getFeatureCategories(featureName); + const categories = dataset.getFeatureCategories(featureKey); if (categories !== null) { hoveredFeatureValue = categories[Number.parseInt(featureVal, 10)]; } else { @@ -716,7 +717,7 @@ function Viewer(): ReactElement { // Stop playback when exporting onClick={() => timeControls.pause()} currentFrame={currentFrame} - defaultImagePrefix={datasetKey + "-" + featureName} + defaultImagePrefix={datasetKey + "-" + featureKey} disabled={dataset === null} setIsRecording={setIsRecording} /> @@ -744,10 +745,10 @@ function Viewer(): ReactElement { { - if (value !== featureName && dataset) { + if (value !== featureKey && dataset) { replaceFeature(dataset, value); resetColorRampRangeToDefaults(dataset, value); } @@ -766,8 +767,8 @@ function Viewer(): ReactElement { disabled={disableUi} knownCategoricalPalettes={KNOWN_CATEGORICAL_PALETTES} categoricalPalettesToDisplay={DISPLAY_CATEGORICAL_PALETTE_KEYS} - useCategoricalPalettes={dataset?.isFeatureCategorical(featureName) || false} - numCategories={dataset?.getFeatureCategories(featureName)?.length || 1} + useCategoricalPalettes={dataset?.isFeatureCategorical(featureKey) || false} + numCategories={dataset?.getFeatureCategories(featureKey)?.length || 1} selectedPalette={categoricalPalette} onChangePalette={setCategoricalPalette} /> @@ -780,15 +781,15 @@ function Viewer(): ReactElement {

- {dataset ? dataset.getFeatureNameWithUnits(featureName) : "Feature value range"} + {dataset ? dataset.getFeatureNameWithUnits(featureKey) : "Feature value range"}

{ // Render either a categorical color picker or a range slider depending on the feature type - dataset?.isFeatureCategorical(featureName) ? ( + dataset?.isFeatureCategorical(featureKey) ? (

Track ID: {lastHoveredId && dataset?.getTrackId(lastHoveredId)}

- {featureName}: {hoveredFeatureValue} + {dataset?.getFeatureName(featureKey) || "Feature"}:{" "} + {hoveredFeatureValue}

} @@ -945,7 +947,7 @@ function Viewer(): ReactElement { findTrack={findTrack} currentFrame={currentFrame} dataset={dataset} - featureName={featureName} + featureKey={featureKey} selectedTrack={selectedTrack} disabled={disableUi} /> @@ -965,7 +967,7 @@ function Viewer(): ReactElement { setFrame={setFrameAndRender} isVisible={config.openTab === TabType.SCATTER_PLOT} isPlaying={timeControls.isPlaying() || isRecording} - selectedFeatureName={featureName} + selectedFeatureKey={featureKey} colorRampMin={colorRampMin} colorRampMax={colorRampMax} colorRamp={getColorMap(colorRampData, colorRampKey, colorRampReversed)} diff --git a/src/colorizer/ColorizeCanvas.ts b/src/colorizer/ColorizeCanvas.ts index 4a06d021a..30a38b03e 100644 --- a/src/colorizer/ColorizeCanvas.ts +++ b/src/colorizer/ColorizeCanvas.ts @@ -135,7 +135,7 @@ export default class ColorizeCanvas { private points: Float32Array; private canvasResolution: Vector2 | null; - private featureName: string | null; + private featureKey: string | null; private selectedBackdropKey: string | null; private colorRamp: ColorRamp; private colorMapRangeMin: number; @@ -191,7 +191,7 @@ export default class ColorizeCanvas { this.dataset = null; this.canvasResolution = null; - this.featureName = null; + this.featureKey = null; this.selectedBackdropKey = null; this.colorRamp = new ColorRamp(["black"]); this.categoricalPalette = new ColorRamp(["black"]); @@ -448,12 +448,12 @@ export default class ColorizeCanvas { this.setUniform("highlightedId", this.track.getIdAtTime(this.currentFrame)); } - setFeature(name: string): void { - if (!this.dataset?.hasFeature(name)) { + setFeatureKey(key: string): void { + if (!this.dataset?.hasFeatureKey(key)) { return; } - const featureData = this.dataset.getFeatureData(name)!; - this.featureName = name; + const featureData = this.dataset.getFeatureData(key)!; + this.featureKey = key; this.setUniform("featureData", featureData.tex); this.render(); // re-render necessary because map range may have changed } @@ -467,10 +467,10 @@ export default class ColorizeCanvas { } resetColorMapRange(): void { - if (!this.featureName) { + if (!this.featureKey) { return; } - const featureData = this.dataset?.getFeatureData(this.featureName); + const featureData = this.dataset?.getFeatureData(this.featureKey); if (featureData) { this.colorMapRangeMin = featureData.min; this.colorMapRangeMax = featureData.max; @@ -575,7 +575,7 @@ export default class ColorizeCanvas { * selected feature. */ updateRamp(): void { - if (this.featureName && this.dataset?.isFeatureCategorical(this.featureName)) { + if (this.featureKey && this.dataset?.isFeatureCategorical(this.featureKey)) { this.setUniform("colorRamp", this.categoricalPalette.texture); this.setUniform("featureColorRampMin", 0); this.setUniform("featureColorRampMax", MAX_FEATURE_CATEGORIES - 1); diff --git a/src/colorizer/Dataset.ts b/src/colorizer/Dataset.ts index e1ba1c1a7..94e57d741 100644 --- a/src/colorizer/Dataset.ts +++ b/src/colorizer/Dataset.ts @@ -130,6 +130,7 @@ export default class Dataset { */ private async loadFeature(metadata: ManifestFile["features"][number]): Promise<[string, FeatureData]> { const name = metadata.name; + const key = metadata.key || getKeyFromName(name); const url = this.resolveUrl(metadata.data); const source = await this.arrayLoader.load(url); const featureType = this.parseFeatureType(metadata.type); @@ -146,10 +147,10 @@ export default class Dataset { } return [ - name, + key, { name, - key: metadata.key || getKeyFromName(name), + key, tex: source.getTexture(FeatureDataType.F32), data: source.getBuffer(FeatureDataType.F32), min: source.getMin(), @@ -161,44 +162,55 @@ export default class Dataset { ]; } - public hasFeature(name: string): boolean { - return this.featureNames.includes(name); + public hasFeatureKey(key: string): boolean { + return this.featureKeys.includes(key); + } + + public findFeatureKeyFromName(name: string): string | undefined { + // TODO: Replace once you add back in featureNames + return Array.from(this.features.values()).find((f) => f.name === name)?.key; } /** * Attempts to get the feature data from this dataset for the given feature name. * Returns `undefined` if feature is not in the dataset. */ - public getFeatureData(name: string): FeatureData | undefined { - return this.features.get(name); + public getFeatureData(key: string): FeatureData | undefined { + return this.features.get(key); } - public getFeatureNameWithUnits(name: string): string { - const units = this.getFeatureUnits(name); - if (units) { - return `${name} (${units})`; - } else { - return name; - } + // TODO: Throw an error if undefined? + public getFeatureName(key: string): string { + return this.features.get(key)!.name; } /** * Gets the feature's units if it exists; otherwise returns an empty string. */ - public getFeatureUnits(name: string): string { - return this.getFeatureData(name)?.units || ""; + public getFeatureUnits(key: string): string { + return this.getFeatureData(key)?.units || ""; + } + + public getFeatureNameWithUnits(key: string): string { + const name = this.getFeatureName(key); + const units = this.getFeatureUnits(key); + if (units) { + return `${name} (${units})`; + } else { + return name; + } } /** * Returns the FeatureType of the given feature, if it exists. - * @param name Feature name to retrieve + * @param key Feature key to retrieve * @throws An error if the feature does not exist. * @returns The FeatureType of the given feature (categorical, continuous, or discrete) */ - public getFeatureType(name: string): FeatureType { - const featureData = this.getFeatureData(name); + public getFeatureType(key: string): FeatureType { + const featureData = this.getFeatureData(key); if (featureData === undefined) { - throw new Error("Feature '" + name + "' does not exist in dataset."); + throw new Error("Feature '" + key + "' does not exist in dataset."); } return featureData.type; } @@ -208,10 +220,10 @@ export default class Dataset { * @param name Feature name to retrieve. * @returns The array of string categories for the given feature, or null if the feature is not categorical. */ - public getFeatureCategories(name: string): string[] | null { - const featureData = this.getFeatureData(name); + public getFeatureCategories(key: string): string[] | null { + const featureData = this.getFeatureData(key); if (featureData === undefined) { - throw new Error("Feature '" + name + "' does not exist in dataset."); + throw new Error("Feature '" + key + "' does not exist in dataset."); } if (featureData.type === FeatureType.CATEGORICAL) { return featureData.categories; @@ -220,8 +232,8 @@ export default class Dataset { } /** Returns whether the given feature represents categorical data. */ - public isFeatureCategorical(name: string): boolean { - const featureData = this.getFeatureData(name); + public isFeatureCategorical(key: string): boolean { + const featureData = this.getFeatureData(key); return featureData !== undefined && featureData.type === FeatureType.CATEGORICAL; } @@ -252,12 +264,16 @@ export default class Dataset { return this.frames?.length || 0; } - public get featureNames(): string[] { + public get featureKeys(): string[] { return Array.from(this.features.keys()); } + // public get featureNames(): string[] { + // return Array.from(this.features.values()).map((f) => f.name); + // } + public get numObjects(): number { - const featureData = this.getFeatureData(this.featureNames[0]); + const featureData = this.getFeatureData(this.featureKeys[0]); if (!featureData) { throw new Error("Dataset.numObjects: The first feature could not be loaded. Is the dataset manifest file valid?"); } @@ -368,8 +384,8 @@ export default class Dataset { this.bounds = bounds; // Keep original sorting order of features by inserting in promise order. - featureResults.forEach(([name, data]) => { - this.features.set(name, data); + featureResults.forEach(([key, data]) => { + this.features.set(key, data); }); if (this.features.size === 0) { diff --git a/src/colorizer/types.ts b/src/colorizer/types.ts index c51516fd9..ac98810c3 100644 --- a/src/colorizer/types.ts +++ b/src/colorizer/types.ts @@ -104,7 +104,7 @@ export enum ThresholdType { type BaseFeatureThreshold = { // TODO: Replace with key string // featureKey: string; - featureName: string; + featureKey: string; units: string; type: ThresholdType; }; diff --git a/src/colorizer/utils/data_utils.ts b/src/colorizer/utils/data_utils.ts index 618400625..07a6ca6fd 100644 --- a/src/colorizer/utils/data_utils.ts +++ b/src/colorizer/utils/data_utils.ts @@ -7,7 +7,7 @@ import Dataset, { FeatureType } from "../Dataset"; /** * Generates a find function for a FeatureThreshold, matching on feature name and unit. - * @param featureName String feature name to match on. + * @param featureKey String feature key to match on. * @param unit String unit to match on. * @returns a lambda function that can be passed into `Array.find` for an array of FeatureThreshold. * @example @@ -16,8 +16,8 @@ import Dataset, { FeatureType } from "../Dataset"; * const matchThreshold = featureThresholds.find(thresholdMatchFinder("Temperature", "°C")); * ``` */ -export function thresholdMatchFinder(featureName: string, units: string): (threshold: FeatureThreshold) => boolean { - return (threshold: FeatureThreshold) => threshold.featureName === featureName && threshold.units === units; +export function thresholdMatchFinder(featureKey: string, units: string): (threshold: FeatureThreshold) => boolean { + return (threshold: FeatureThreshold) => threshold.featureKey === featureKey && threshold.units === units; } /** @@ -33,7 +33,7 @@ export function getColorMap(colorRampData: Map, key: stri /** * Validates the thresholds against the dataset. If the threshold's feature is present but the wrong type, it is updated. - * This is most likely to happen when datasets have different types for the same feature name, or if thresholds are loaded from + * This is most likely to happen when datasets have different types for the same feature key, or if thresholds are loaded from * an outdated URL. * @param dataset The dataset to validate thresholds against. * @param thresholds An array of `FeatureThresholds` to validate. @@ -45,7 +45,7 @@ export function validateThresholds(dataset: Dataset, thresholds: FeatureThreshol const newThresholds: FeatureThreshold[] = []; for (const threshold of thresholds) { - const featureData = dataset.getFeatureData(threshold.featureName); + const featureData = dataset.getFeatureData(threshold.featureKey); const isInDataset = featureData && featureData.units === threshold.units; if (isInDataset && featureData.type === FeatureType.CATEGORICAL && isThresholdNumeric(threshold)) { @@ -55,7 +55,7 @@ export function validateThresholds(dataset: Dataset, thresholds: FeatureThreshol // This is important for historical reasons, because older versions of the app used to only store features as numeric // thresholds. This would cause categorical features loaded from the URL to be incorrectly shown on the UI. newThresholds.push({ - featureName: threshold.featureName, + featureKey: threshold.featureKey, units: threshold.units, type: ThresholdType.CATEGORICAL, enabledCategories: Array(MAX_FEATURE_CATEGORIES).fill(true), @@ -64,7 +64,7 @@ export function validateThresholds(dataset: Dataset, thresholds: FeatureThreshol // Threshold is categorical but the feature is not. // Convert to numeric threshold instead. newThresholds.push({ - featureName: threshold.featureName, + featureKey: threshold.featureKey, units: threshold.units, type: ThresholdType.NUMERIC, min: featureData.min, @@ -91,8 +91,8 @@ export function isValueWithinThreshold(value: number, threshold: FeatureThreshol * Returns a Uint8 array look-up table indexed by object ID, storing whether that object ID is in range of * the given thresholds (=1) or not (=0). * @param {Dataset} dataset A valid Dataset object. - * @param {FeatureThreshold[]} thresholds Array of feature thresholds, which match agaisnt the feature name and unit. - * If a feature name cannot be found in the dataset, it will be ignored. + * @param {FeatureThreshold[]} thresholds Array of feature thresholds, which match agaisnt the feature key and unit. + * If a feature key cannot be found in the dataset, it will be ignored. * @returns A Uint8Array, with a length equal to the number of objects in the dataset. * For each object ID `i`, `inRangeIds[i]` will be 1 if the object is in range of the thresholds * and 0 if it is not. @@ -104,7 +104,7 @@ export function getInRangeLUT(dataset: Dataset, thresholds: FeatureThreshold[]): // Ignore thresholds with features that don't exist in this dataset or whose units don't match const validThresholds = thresholds.filter((threshold) => { - const featureData = dataset.getFeatureData(threshold.featureName); + const featureData = dataset.getFeatureData(threshold.featureKey); return featureData && featureData.units === threshold.units; }); @@ -112,7 +112,7 @@ export function getInRangeLUT(dataset: Dataset, thresholds: FeatureThreshold[]): inRangeIds[id] = 1; for (let thresholdIdx = 0; thresholdIdx < validThresholds.length; thresholdIdx++) { const threshold = validThresholds[thresholdIdx]; - const featureData = dataset.getFeatureData(threshold.featureName); + const featureData = dataset.getFeatureData(threshold.featureKey); if (featureData && !isValueWithinThreshold(featureData.data[id], threshold)) { inRangeIds[id] = 0; break; diff --git a/src/colorizer/utils/url_utils.ts b/src/colorizer/utils/url_utils.ts index f1ee253ff..809389b84 100644 --- a/src/colorizer/utils/url_utils.ts +++ b/src/colorizer/utils/url_utils.ts @@ -26,7 +26,9 @@ import { numberToStringDecimal } from "./math_utils"; enum UrlParam { TRACK = "track", DATASET = "dataset", + // Will be deprecated once feature keys are implemented FEATURE = "feature", + FEATURE_KEY = "feature-key", TIME = "t", COLLECTION = "collection", THRESHOLDS = "filters", @@ -64,7 +66,9 @@ const ALLEN_PREFIX_TO_HTTPS: Record = { export type UrlParams = { collection: string; dataset: string; + /** Will be deprecated. */ feature: string; + featureKey: string; track: number; time: number; thresholds: FeatureThreshold[]; @@ -113,7 +117,7 @@ export function fetchWithTimeout( function serializeThreshold(threshold: FeatureThreshold): string { // featureName + units are encoded in case it contains special characters (":" or ","). // TODO: remove once feature keys are implemented. - const featureName = encodeURIComponent(threshold.featureName); + const featureName = encodeURIComponent(threshold.featureKey); const featureUnit = encodeURIComponent(threshold.units); // TODO: Are there better characters I can be using here? ":" and "," take up @@ -142,8 +146,8 @@ function serializeThreshold(threshold: FeatureThreshold): string { * - `undefined` if the string could not be parsed. */ function deserializeThreshold(thresholdString: string): FeatureThreshold | undefined { - const [featureName, featureUnit, ...selection] = thresholdString.split(":"); - if (featureName === undefined || featureUnit === undefined) { + const [featureKey, featureUnit, ...selection] = thresholdString.split(":"); + if (featureKey === undefined || featureUnit === undefined) { console.warn( "url_utils.deserializeThreshold: Could not parse threshold string: '" + thresholdString + @@ -161,7 +165,7 @@ function deserializeThreshold(thresholdString: string): FeatureThreshold | undef enabledCategories.push((selectedBinary & (1 << i)) !== 0); } threshold = { - featureName: decodeURIComponent(featureName), + featureKey: decodeURIComponent(featureKey), units: decodeURIComponent(featureUnit), type: ThresholdType.CATEGORICAL, enabledCategories, @@ -169,7 +173,7 @@ function deserializeThreshold(thresholdString: string): FeatureThreshold | undef } else if (selection.length === 2) { // Feature is numeric and a range. threshold = { - featureName: decodeURIComponent(featureName), + featureKey: decodeURIComponent(featureKey), units: decodeURIComponent(featureUnit), type: ThresholdType.NUMERIC, min: parseFloat(selection[0]), @@ -185,7 +189,7 @@ function deserializeThreshold(thresholdString: string): FeatureThreshold | undef `url_utils.deserializeThreshold: invalid threshold '${thresholdString}' has too many or too few parameters.` ); threshold = { - featureName: decodeURIComponent(featureName), + featureKey: decodeURIComponent(featureKey), units: decodeURIComponent(featureUnit), type: ThresholdType.NUMERIC, min: NaN, @@ -365,6 +369,7 @@ function deserializeScatterPlotConfig(params: URLSearchParams): Partial): string { if (state.feature) { includedParameters.push(`${UrlParam.FEATURE}=${encodeURIComponent(state.feature)}`); } + if (state.featureKey) { + includedParameters.push(`${UrlParam.FEATURE_KEY}=${encodeURIComponent(state.featureKey)}`); + } if (state.track !== undefined) { includedParameters.push(`${UrlParam.TRACK}=${state.track}`); } @@ -539,6 +547,7 @@ export function loadFromUrlSearchParams(urlParams: URLSearchParams): Partial = {}; @@ -44,9 +44,9 @@ export default function PlotWrapper(inputProps: PlotWrapperProps): ReactElement useMemo(() => { plot?.removePlot(); if (props.selectedTrack) { - plot?.plot(props.selectedTrack, props.featureName, props.frame); + plot?.plot(props.selectedTrack, props.dataset?.getFeatureName(props.featureKey) || "", props.frame); } - }, [props.selectedTrack, props.featureName]); + }, [props.selectedTrack, props.featureKey]); const updatePlotSize = (): void => { if (!plotDivRef.current) { diff --git a/src/components/Tabs/PlotTab.tsx b/src/components/Tabs/PlotTab.tsx index a2e67998d..e72d464d5 100644 --- a/src/components/Tabs/PlotTab.tsx +++ b/src/components/Tabs/PlotTab.tsx @@ -30,7 +30,7 @@ type PlotTabProps = { findTrack: (trackId: number, seekToFrame?: boolean) => void; currentFrame: number; dataset: Dataset | null; - featureName: string; + featureKey: string; selectedTrack: Track | null; disabled: boolean; }; @@ -66,7 +66,7 @@ export default function PlotTab(props: PlotTabProps): ReactElement { diff --git a/src/components/Tabs/ScatterPlotTab.tsx b/src/components/Tabs/ScatterPlotTab.tsx index a080a30dd..92e596cdb 100644 --- a/src/components/Tabs/ScatterPlotTab.tsx +++ b/src/components/Tabs/ScatterPlotTab.tsx @@ -55,7 +55,7 @@ type ScatterPlotTabProps = { isVisible: boolean; isPlaying: boolean; - selectedFeatureName: string | null; + selectedFeatureKey: string | null; colorRampMin: number; colorRampMax: number; colorRamp: ColorRamp; @@ -116,7 +116,7 @@ export default memo(function ScatterPlotTab(props: ScatterPlotTabProps): ReactEl currentFrame, colorRamp, categoricalPalette, - selectedFeatureName, + selectedFeatureKey, isPlaying, isVisible, inRangeIds, @@ -416,16 +416,16 @@ export default memo(function ScatterPlotTab(props: ScatterPlotTabProps): ReactEl markerConfig: Partial & { outliers?: Partial; outOfRange?: Partial } = {}, overrideColor?: Color ): Partial[] => { - if (selectedFeatureName === null || dataset === null || !xAxisFeatureName || !yAxisFeatureName) { + if (selectedFeatureKey === null || dataset === null || !xAxisFeatureName || !yAxisFeatureName) { return []; } - const featureData = dataset.getFeatureData(selectedFeatureName); + const featureData = dataset.getFeatureData(selectedFeatureKey); if (!featureData) { return []; } // Generate colors - const categories = dataset.getFeatureCategories(selectedFeatureName); + const categories = dataset.getFeatureCategories(selectedFeatureKey); const isCategorical = categories !== null; const usingOverrideColor = markerConfig.color || overrideColor; overrideColor = overrideColor || new Color(markerConfig.color as ColorRepresentation); @@ -569,7 +569,7 @@ export default memo(function ScatterPlotTab(props: ScatterPlotTabProps): ReactEl isPlaying, plotDivRef.current, viewerConfig, - selectedFeatureName, + selectedFeatureKey, colorRampMin, colorRampMax, colorRamp, @@ -742,9 +742,9 @@ export default memo(function ScatterPlotTab(props: ScatterPlotTabProps): ReactEl ////////////////////////////////// const makeControlBar = (): ReactElement => { - const featureNames = dataset?.featureNames || []; - const menuItems: MenuItemType[] = featureNames.map((name: string) => { - return { key: name, label: dataset?.getFeatureNameWithUnits(name) }; + const featureKeys = dataset ? dataset.featureKeys : []; + const menuItems: MenuItemType[] = featureKeys.map((key: string) => { + return { key, label: dataset?.getFeatureNameWithUnits(key) }; }); menuItems.push({ key: TIME_FEATURE.name, label: TIME_FEATURE.name }); diff --git a/tests/Dataset.test.ts b/tests/Dataset.test.ts index ed101dca2..622a16d13 100644 --- a/tests/Dataset.test.ts +++ b/tests/Dataset.test.ts @@ -88,11 +88,17 @@ describe("Dataset", () => { const manifestV1_0_0: AnyManifestFile = { ...manifestV0_0_0, features: [ - { name: "feature1", data: "feature1.json", units: "meters", type: "continuous" }, - { name: "feature2", data: "feature2.json", units: "(m)", type: "discrete" }, - { name: "feature3", data: "feature3.json", units: "μm/s", type: "bad-type" }, - { name: "feature4", data: "feature4.json" }, - { name: "feature5", data: "feature4.json", type: "categorical", categories: ["small", "medium", "large"] }, + { key: "feature1", name: "Feature1", data: "feature1.json", units: "meters", type: "continuous" }, + { key: "feature2", name: "Feature2", data: "feature2.json", units: "(m)", type: "discrete" }, + { key: "feature3", name: "Feature3", data: "feature3.json", units: "μm/s", type: "bad-type" }, + { key: "feature4", name: "Feature4", data: "feature4.json" }, + { + key: "feature5", + name: "Feature5", + data: "feature4.json", + type: "categorical", + categories: ["small", "medium", "large"], + }, ], metadata: { frameDims: { @@ -131,6 +137,10 @@ describe("Dataset", () => { // Test both normal and deprecated manifests for (const [version, manifest] of manifestsToTest) { describe(version, () => { + it("fills in feature keys if only names are provided", () => { + throw new Error("Test not implemented"); + }); + it("retrieves feature units", async () => { const dataset = new Dataset(defaultPath, new MockFrameLoader(), new MockArrayLoader()); const mockFetch = makeMockFetchMethod(defaultPath, manifest); From 8ccb9876f4132a88c34f28b76ed2fcb9b8f4797f Mon Sep 17 00:00:00 2001 From: Peyton Lee Date: Fri, 22 Mar 2024 17:01:04 -0700 Subject: [PATCH 04/16] fix: Fixed plot, scatter plot, filters tab --- src/colorizer/Dataset.ts | 14 +-- src/colorizer/Plotting.ts | 6 +- src/components/PlotWrapper.tsx | 2 +- src/components/Tabs/FeatureThresholdsTab.tsx | 42 ++++----- src/components/Tabs/ScatterPlotTab.tsx | 92 +++++++++---------- .../Tabs/scatter_plot_data_utils.ts | 6 +- 6 files changed, 81 insertions(+), 81 deletions(-) diff --git a/src/colorizer/Dataset.ts b/src/colorizer/Dataset.ts index 94e57d741..3e09bf62c 100644 --- a/src/colorizer/Dataset.ts +++ b/src/colorizer/Dataset.ts @@ -60,7 +60,7 @@ export default class Dataset { private arrayLoader: IArrayLoader; // Use map to enforce ordering - /** Maps, in order, from feature names to feature data. */ + /** Maps, in order, from feature keys to feature data. */ private features: Map; private outlierFile?: string; @@ -126,7 +126,7 @@ export default class Dataset { /** * Loads a feature from the dataset, fetching its data from the provided url. - * @returns A promise of an array tuple containing the feature name and its FeatureData. + * @returns A promise of an array tuple containing the feature key and its FeatureData. */ private async loadFeature(metadata: ManifestFile["features"][number]): Promise<[string, FeatureData]> { const name = metadata.name; @@ -172,7 +172,7 @@ export default class Dataset { } /** - * Attempts to get the feature data from this dataset for the given feature name. + * Attempts to get the feature data from this dataset for the given feature key. * Returns `undefined` if feature is not in the dataset. */ public getFeatureData(key: string): FeatureData | undefined { @@ -217,7 +217,7 @@ export default class Dataset { /** * Returns the array of string categories for the given feature, if it exists and is categorical. - * @param name Feature name to retrieve. + * @param key Feature key to retrieve. * @returns The array of string categories for the given feature, or null if the feature is not categorical. */ public getFeatureCategories(key: string): string[] | null { @@ -450,10 +450,10 @@ export default class Dataset { * Get the times and values of a track for a given feature * this data is suitable to hand to d3 or plotly as two arrays of domain and range values */ - public buildTrackFeaturePlot(track: Track, feature: string): { domain: number[]; range: number[] } { - const featureData = this.getFeatureData(feature); + public buildTrackFeaturePlot(track: Track, featureKey: string): { domain: number[]; range: number[] } { + const featureData = this.getFeatureData(featureKey); if (!featureData) { - throw new Error("Dataset.buildTrackFeaturePlot: Feature '" + feature + "' does not exist in dataset."); + throw new Error("Dataset.buildTrackFeaturePlot: Feature '" + featureKey + "' does not exist in dataset."); } const range = track.ids.map((i) => featureData.data[i]); const domain = track.times; diff --git a/src/colorizer/Plotting.ts b/src/colorizer/Plotting.ts index 1c1fe68c2..a2390d084 100644 --- a/src/colorizer/Plotting.ts +++ b/src/colorizer/Plotting.ts @@ -54,11 +54,11 @@ export default class Plotting { this.dataset = dataset; } - plot(track: Track, feature: string, time: number): void { + plot(track: Track, featureKey: string, time: number): void { if (!this.dataset) { return; } - const plotinfo = this.dataset?.buildTrackFeaturePlot(track, feature); + const plotinfo = this.dataset?.buildTrackFeaturePlot(track, featureKey); this.trace = { x: plotinfo.domain, y: plotinfo.range, @@ -67,7 +67,7 @@ export default class Plotting { const layout: Partial = { yaxis: { - title: this.dataset.getFeatureNameWithUnits(feature), + title: this.dataset.getFeatureNameWithUnits(featureKey), }, shapes: [ { diff --git a/src/components/PlotWrapper.tsx b/src/components/PlotWrapper.tsx index 948857bef..a81843431 100644 --- a/src/components/PlotWrapper.tsx +++ b/src/components/PlotWrapper.tsx @@ -44,7 +44,7 @@ export default function PlotWrapper(inputProps: PlotWrapperProps): ReactElement useMemo(() => { plot?.removePlot(); if (props.selectedTrack) { - plot?.plot(props.selectedTrack, props.dataset?.getFeatureName(props.featureKey) || "", props.frame); + plot?.plot(props.selectedTrack, props.featureKey, props.frame); } }, [props.selectedTrack, props.featureKey]); diff --git a/src/components/Tabs/FeatureThresholdsTab.tsx b/src/components/Tabs/FeatureThresholdsTab.tsx index bd219a5a9..1fa6bf016 100644 --- a/src/components/Tabs/FeatureThresholdsTab.tsx +++ b/src/components/Tabs/FeatureThresholdsTab.tsx @@ -132,7 +132,7 @@ export default function FeatureThresholdsTab(inputProps: FeatureThresholdsTabPro /** Converts a threshold to a unique key that can be used to look up its information later. Matches on feature name and unit. */ const thresholdToKey = (threshold: FeatureThreshold): string => { - return `${encodeURIComponent(threshold.featureName)}:${threshold.units ? encodeURIComponent(threshold.units) : ""}`; + return `${encodeURIComponent(threshold.featureKey)}:${threshold.units ? encodeURIComponent(threshold.units) : ""}`; }; // Save the FEATURE min/max bounds (not the selected range of the threshold) for each threshold. We do // this in case the user switches to a dataset that no longer has the threshold's feature @@ -147,7 +147,7 @@ export default function FeatureThresholdsTab(inputProps: FeatureThresholdsTabPro // that has the same feature/unit but different min/max values. That way, our saved min/max bounds // reflect the last known good values. for (const threshold of props.featureThresholds) { - const featureData = props.dataset?.getFeatureData(threshold.featureName); + const featureData = props.dataset?.getFeatureData(threshold.featureKey); if (featureData && featureData.units === threshold.units && featureData.type !== FeatureType.CATEGORICAL) { featureMinMaxBoundsFallback.current.set(thresholdToKey(threshold), [featureData.min, featureData.max]); } @@ -157,15 +157,15 @@ export default function FeatureThresholdsTab(inputProps: FeatureThresholdsTabPro ////// EVENT HANDLERS /////////////////// /** Handle the user selecting new features from the Select dropdown. */ - const onSelect = (featureName: string): void => { - const featureData = props.dataset?.getFeatureData(featureName); + const onSelect = (featureKey: string): void => { + const featureData = props.dataset?.getFeatureData(featureKey); const newThresholds = [...props.featureThresholds]; - if (featureData && !props.dataset?.isFeatureCategorical(featureName)) { + if (featureData && !props.dataset?.isFeatureCategorical(featureKey)) { // Continuous/discrete feature newThresholds.push({ type: ThresholdType.NUMERIC, - featureName: featureName, - units: props.dataset!.getFeatureUnits(featureName), + featureKey: featureKey, + units: props.dataset!.getFeatureUnits(featureKey), min: featureData.min, max: featureData.max, }); @@ -173,8 +173,8 @@ export default function FeatureThresholdsTab(inputProps: FeatureThresholdsTabPro // Categorical feature newThresholds.push({ type: ThresholdType.CATEGORICAL, - featureName: featureName, - units: props.dataset!.getFeatureUnits(featureName), + featureKey: featureKey, + units: props.dataset!.getFeatureUnits(featureKey), enabledCategories: Array(MAX_FEATURE_CATEGORIES).fill(true), }); } @@ -182,12 +182,12 @@ export default function FeatureThresholdsTab(inputProps: FeatureThresholdsTabPro }; /** Handle the user removing features from the Select dropdown. */ - const onDeselect = (featureName: string): void => { + const onDeselect = (featureKey: string): void => { // Find the exact match for the threshold and remove it - const featureData = props.dataset?.getFeatureData(featureName); + const featureData = props.dataset?.getFeatureData(featureKey); const newThresholds = [...props.featureThresholds]; if (featureData) { - const index = props.featureThresholds.findIndex(thresholdMatchFinder(featureName, featureData.units)); + const index = props.featureThresholds.findIndex(thresholdMatchFinder(featureKey, featureData.units)); if (index !== -1) { // Delete saved min/max bounds for this feature const thresholdToRemove = props.featureThresholds[index]; @@ -239,20 +239,20 @@ export default function FeatureThresholdsTab(inputProps: FeatureThresholdsTabPro ////// RENDERING /////////////////// // The Select dropdown should ONLY show features that are currently present in the dataset. const featureOptions = - props.dataset?.featureNames.map((name) => ({ - label: props.dataset?.getFeatureNameWithUnits(name), - value: name, + props.dataset?.featureKeys.map((key) => ({ + label: props.dataset?.getFeatureNameWithUnits(key), + value: key, })) || []; // Filter out thresholds that no longer match the dataset (feature and/or unit), so we only // show selections that are actually valid. const thresholdsInDataset = props.featureThresholds.filter((t) => { - const featureData = props.dataset?.getFeatureData(t.featureName); + const featureData = props.dataset?.getFeatureData(t.featureKey); return featureData && featureData.units === t.units; }); - const selectedFeatures = thresholdsInDataset.map((t) => t.featureName); + const selectedFeatures = thresholdsInDataset.map((t) => t.featureKey); const renderNumericItem = (item: NumericFeatureThreshold, index: number): ReactNode => { - const featureData = props.dataset?.getFeatureData(item.featureName); + const featureData = props.dataset?.getFeatureData(item.featureKey); const disabled = featureData === undefined || featureData.units !== item.units; // If the feature is no longer in the dataset, use the saved min/max bounds. const savedMinMax = featureMinMaxBoundsFallback.current.get(thresholdToKey(item)) || [Number.NaN, Number.NaN]; @@ -274,7 +274,7 @@ export default function FeatureThresholdsTab(inputProps: FeatureThresholdsTabPro }; const renderCategoricalItem = (item: CategoricalFeatureThreshold, index: number): ReactNode => { - const featureData = props.dataset?.getFeatureData(item.featureName); + const featureData = props.dataset?.getFeatureData(item.featureKey); const disabled = featureData === undefined || featureData.units !== item.units; const categories = featureData?.categories || []; @@ -307,9 +307,9 @@ export default function FeatureThresholdsTab(inputProps: FeatureThresholdsTabPro const renderListItems = (threshold: FeatureThreshold, index: number): ReactNode => { // Thresholds are matched on both feature names and units; a threshold must match // both in the current dataset to be enabled and editable. - const featureData = props.dataset?.getFeatureData(threshold.featureName); + const featureData = props.dataset?.getFeatureData(threshold.featureKey); const disabled = featureData === undefined || featureData.units !== threshold.units; - const featureLabel = threshold.units ? `${threshold.featureName} (${threshold.units})` : threshold.featureName; + const featureLabel = threshold.units ? `${threshold.featureKey} (${threshold.units})` : threshold.featureKey; return ( diff --git a/src/components/Tabs/ScatterPlotTab.tsx b/src/components/Tabs/ScatterPlotTab.tsx index 92e596cdb..7eb39c5b4 100644 --- a/src/components/Tabs/ScatterPlotTab.tsx +++ b/src/components/Tabs/ScatterPlotTab.tsx @@ -30,7 +30,7 @@ import IconButton from "../IconButton"; import LoadingSpinner from "../LoadingSpinner"; /** Extra feature that's added to the dropdowns representing the frame number. */ -const TIME_FEATURE = { key: "scatterplot_time", name: "Time" }; +const TIME_FEATURE = { key: "scatterplot_time", label: "Time" }; // TODO: Translate into seconds/minutes/hours for datasets where frame duration is known? const PLOTLY_CONFIG: Partial = { @@ -97,8 +97,8 @@ export default memo(function ScatterPlotTab(props: ScatterPlotTabProps): ReactEl [], { autosize: true, - xaxis: { title: xAxisFeatureName || "" }, - yaxis: { title: yAxisFeatureName || "" }, + xaxis: { title: xAxisFeatureKey || "" }, + yaxis: { title: yAxisFeatureKey || "" }, }, PLOTLY_CONFIG ).then((plot) => { @@ -144,7 +144,7 @@ export default memo(function ScatterPlotTab(props: ScatterPlotTabProps): ReactEl }); } }, [props.scatterPlotConfig]); - const { xAxis: xAxisFeatureName, yAxis: yAxisFeatureName, rangeType } = scatterConfig; + const { xAxis: xAxisFeatureKey, yAxis: yAxisFeatureKey, rangeType } = scatterConfig; const isDebouncePending = props.scatterPlotConfig !== scatterConfig || @@ -189,27 +189,27 @@ export default memo(function ScatterPlotTab(props: ScatterPlotTabProps): ReactEl ////////////////////////////////// /** Retrieve feature data, if it exists. Accounts for the artificially-added time feature. */ - const getData = (featureName: string | null, dataset: Dataset | null): Uint32Array | Float32Array | undefined => { - if (featureName === null || dataset === null) { + const getData = (featureKey: string | null, dataset: Dataset | null): Uint32Array | Float32Array | undefined => { + if (featureKey === null || dataset === null) { return undefined; } - if (featureName === TIME_FEATURE.name) { + if (featureKey === TIME_FEATURE.key) { return dataset.times || undefined; } - return dataset.getFeatureData(featureName)?.data; + return dataset.getFeatureData(featureKey)?.data; }; // Track last rendered props + state to make optimizations on re-renders type LastRenderedState = { rangeType: PlotRangeType; - xAxisFeatureName: string | null; - yAxisFeatureName: string | null; + xAxisFeatureKey: string | null; + yAxisFeatureKey: string | null; } & ScatterPlotTabProps; const lastRenderedState = useRef({ rangeType: DEFAULT_RANGE_TYPE, - xAxisFeatureName: null, - yAxisFeatureName: null, + xAxisFeatureKey: null, + yAxisFeatureKey: null, ...props, }); @@ -219,7 +219,7 @@ export default memo(function ScatterPlotTab(props: ScatterPlotTabProps): ReactEl // This prevents clicking on a new track from resetting the plot view. const lastState = lastRenderedState.current; const haveAxesChanged = - lastState.xAxisFeatureName !== xAxisFeatureName || lastState.yAxisFeatureName !== yAxisFeatureName; + lastState.xAxisFeatureKey !== xAxisFeatureKey || lastState.yAxisFeatureKey !== yAxisFeatureKey; const hasRangeChanged = lastState.rangeType !== rangeType; const hasDatasetChanged = lastState.dataset !== dataset; return haveAxesChanged || hasRangeChanged || hasDatasetChanged; @@ -305,7 +305,7 @@ export default memo(function ScatterPlotTab(props: ScatterPlotTabProps): ReactEl /** * Creates the scatterplot and histogram axes for a given feature. Normalizes for dataset min/max to * prevents axes from jumping during time or track playback. - * @param featureName Name of the feature to generate layouts for. + * @param featureKey Name of the feature to generate layouts for. * @param histogramTrace The default histogram trace configuration. * @returns An object with the following keys: * - `scatterPlotAxis`: Layout for the scatter plot axis. @@ -313,7 +313,7 @@ export default memo(function ScatterPlotTab(props: ScatterPlotTabProps): ReactEl * - `histogramTrace`: A copy of the histogram trace, with potentially updated bin sizes. */ const getAxisLayoutsFromRange = ( - featureName: string, + featureKey: string, histogramTrace: Partial ): { scatterPlotAxis: Partial; @@ -333,16 +333,16 @@ export default memo(function ScatterPlotTab(props: ScatterPlotTabProps): ReactEl }; const newHistogramTrace = { ...histogramTrace }; - let min = dataset?.getFeatureData(featureName)?.min || 0; - let max = dataset?.getFeatureData(featureName)?.max || 0; + let min = dataset?.getFeatureData(featureKey)?.min || 0; + let max = dataset?.getFeatureData(featureKey)?.max || 0; // Special case for time feature, which isn't in the dataset - if (featureName === TIME_FEATURE.name) { + if (featureKey === TIME_FEATURE.key) { min = 0; max = dataset?.numberOfFrames || 0; } - if (dataset && dataset.isFeatureCategorical(featureName)) { + if (dataset && dataset.isFeatureCategorical(featureKey)) { // Add extra padding for categories so they're nicely centered min -= 0.5; max += 0.5; @@ -358,9 +358,9 @@ export default memo(function ScatterPlotTab(props: ScatterPlotTabProps): ReactEl // TODO: Add special handling for integer features once implemented, so their histograms use reasonable // bin sizes to prevent jumping. - if (dataset && dataset.isFeatureCategorical(featureName)) { + if (dataset && dataset.isFeatureCategorical(featureKey)) { // Create custom tick marks for the categories - const categories = dataset.getFeatureCategories(featureName) || []; + const categories = dataset.getFeatureCategories(featureKey) || []; scatterPlotAxis = { ...scatterPlotAxis, tickmode: "array", @@ -382,11 +382,11 @@ export default memo(function ScatterPlotTab(props: ScatterPlotTabProps): ReactEl /** * VERY roughly estimate the max width in pixels needed for a categorical feature. */ - const estimateTextWidthPxForCategories = (featureName: string): number => { - if (featureName === null || !dataset?.isFeatureCategorical(featureName)) { + const estimateTextWidthPxForCategories = (featureKey: string): number => { + if (featureKey === null || !dataset?.isFeatureCategorical(featureKey)) { return 0; } - const categories = dataset.getFeatureCategories(featureName) || []; + const categories = dataset.getFeatureCategories(featureKey) || []; return ( categories.reduce((_prev: any, val: string, acc: number) => { return Math.max(val.length, acc); @@ -416,7 +416,7 @@ export default memo(function ScatterPlotTab(props: ScatterPlotTabProps): ReactEl markerConfig: Partial & { outliers?: Partial; outOfRange?: Partial } = {}, overrideColor?: Color ): Partial[] => { - if (selectedFeatureKey === null || dataset === null || !xAxisFeatureName || !yAxisFeatureName) { + if (selectedFeatureKey === null || dataset === null || !xAxisFeatureKey || !yAxisFeatureKey) { return []; } const featureData = dataset.getFeatureData(selectedFeatureKey); @@ -547,7 +547,7 @@ export default memo(function ScatterPlotTab(props: ScatterPlotTabProps): ReactEl size: 4, ...bucket.marker, }, - hovertemplate: getHoverTemplate(dataset, xAxisFeatureName, yAxisFeatureName), + hovertemplate: getHoverTemplate(dataset, xAxisFeatureKey, yAxisFeatureKey), }; }); @@ -560,8 +560,8 @@ export default memo(function ScatterPlotTab(props: ScatterPlotTabProps): ReactEl const plotDependencies = [ dataset, - xAxisFeatureName, - yAxisFeatureName, + xAxisFeatureKey, + yAxisFeatureKey, rangeType, currentFrame, selectedTrack, @@ -578,10 +578,10 @@ export default memo(function ScatterPlotTab(props: ScatterPlotTabProps): ReactEl ]; const renderPlot = (forceRelayout: boolean = false): void => { - const rawXData = getData(xAxisFeatureName, dataset); - const rawYData = getData(yAxisFeatureName, dataset); + const rawXData = getData(xAxisFeatureKey, dataset); + const rawYData = getData(yAxisFeatureKey, dataset); - if (!rawXData || !rawYData || !xAxisFeatureName || !yAxisFeatureName || !dataset || !plotDivRef.current) { + if (!rawXData || !rawYData || !xAxisFeatureKey || !yAxisFeatureKey || !dataset || !plotDivRef.current) { clearPlotAndStopRender(); return; } @@ -600,7 +600,7 @@ export default memo(function ScatterPlotTab(props: ScatterPlotTabProps): ReactEl markerBaseColor = new Color("#dddddd"); } - const isUsingTime = xAxisFeatureName === TIME_FEATURE.name || yAxisFeatureName === TIME_FEATURE.name; + const isUsingTime = xAxisFeatureKey === TIME_FEATURE.key || yAxisFeatureKey === TIME_FEATURE.key; // Configure traces const traces = colorizeScatterplotPoints(xData, yData, objectIds, trackIds, {}, markerBaseColor); @@ -632,7 +632,7 @@ export default memo(function ScatterPlotTab(props: ScatterPlotTabProps): ReactEl if (trackData && rangeType !== PlotRangeType.CURRENT_FRAME) { // Render an extra trace for lines connecting the points in the current track when time is a feature. if (isUsingTime) { - const hovertemplate = getHoverTemplate(dataset, xAxisFeatureName, yAxisFeatureName); + const hovertemplate = getHoverTemplate(dataset, xAxisFeatureKey, yAxisFeatureKey); traces.push( makeLineTrace(trackData.xData, trackData.yData, trackData.objectIds, trackData.trackIds, hovertemplate) ); @@ -673,22 +673,22 @@ export default memo(function ScatterPlotTab(props: ScatterPlotTabProps): ReactEl // Format axes const { scatterPlotAxis: scatterPlotXAxis, histogramAxis: histogramXAxis } = getAxisLayoutsFromRange( - xAxisFeatureName, + xAxisFeatureKey, xHistogram ); const { scatterPlotAxis: scatterPlotYAxis, histogramAxis: histogramYAxis } = getAxisLayoutsFromRange( - yAxisFeatureName, + yAxisFeatureKey, yHistogram ); - scatterPlotXAxis.title = dataset.getFeatureNameWithUnits(xAxisFeatureName || ""); + scatterPlotXAxis.title = dataset.getFeatureNameWithUnits(xAxisFeatureKey || ""); // Due to limited space in the Y-axis, hide categorical feature names. - scatterPlotYAxis.title = dataset.isFeatureCategorical(yAxisFeatureName) + scatterPlotYAxis.title = dataset.isFeatureCategorical(yAxisFeatureKey) ? "" - : dataset.getFeatureNameWithUnits(yAxisFeatureName || ""); + : dataset.getFeatureNameWithUnits(yAxisFeatureKey || ""); // Add extra margin for categorical feature labels on the Y axis. - const leftMarginPx = Math.max(60, estimateTextWidthPxForCategories(yAxisFeatureName)); + const leftMarginPx = Math.max(60, estimateTextWidthPxForCategories(yAxisFeatureKey)); const layout = { autosize: true, showlegend: false, @@ -719,8 +719,8 @@ export default memo(function ScatterPlotTab(props: ScatterPlotTabProps): ReactEl Plotly.react(plotDivRef.current, traces, layout, PLOTLY_CONFIG).then(() => { setIsRendering(false); lastRenderedState.current = { - xAxisFeatureName, - yAxisFeatureName, + xAxisFeatureKey, + yAxisFeatureKey, rangeType, ...props, }; @@ -746,13 +746,13 @@ export default memo(function ScatterPlotTab(props: ScatterPlotTabProps): ReactEl const menuItems: MenuItemType[] = featureKeys.map((key: string) => { return { key, label: dataset?.getFeatureNameWithUnits(key) }; }); - menuItems.push({ key: TIME_FEATURE.name, label: TIME_FEATURE.name }); + menuItems.push(TIME_FEATURE); return ( props.updateScatterPlotConfig({ xAxis: key })} /> @@ -760,8 +760,8 @@ export default memo(function ScatterPlotTab(props: ScatterPlotTabProps): ReactEl { props.updateScatterPlotConfig({ - xAxis: yAxisFeatureName, - yAxis: xAxisFeatureName, + xAxis: yAxisFeatureKey, + yAxis: xAxisFeatureKey, }); }} type="link" @@ -771,7 +771,7 @@ export default memo(function ScatterPlotTab(props: ScatterPlotTabProps): ReactEl props.updateScatterPlotConfig({ yAxis: key })} /> diff --git a/src/components/Tabs/scatter_plot_data_utils.ts b/src/components/Tabs/scatter_plot_data_utils.ts index 97b483400..e3e101a6e 100644 --- a/src/components/Tabs/scatter_plot_data_utils.ts +++ b/src/components/Tabs/scatter_plot_data_utils.ts @@ -122,10 +122,10 @@ export function scaleColorOpacityByMarkerCount(numMarkers: number, baseColor: He * Returns a Plotly hovertemplate string for a scatter plot trace. * The trace must include the `id` (object ID) and `customdata` (track ID) fields. */ -export function getHoverTemplate(dataset: Dataset, xAxisFeatureName: string, yAxisFeatureName: string): string { +export function getHoverTemplate(dataset: Dataset, xAxisFeatureKey: string, yAxisFeatureKey: string): string { return ( - `${xAxisFeatureName}: %{x} ${dataset.getFeatureUnits(xAxisFeatureName)}` + - `
${yAxisFeatureName}: %{y} ${dataset.getFeatureUnits(yAxisFeatureName)}` + + `${dataset.getFeatureName(xAxisFeatureKey)}: %{x} ${dataset.getFeatureUnits(xAxisFeatureKey)}` + + `
${dataset.getFeatureName(yAxisFeatureKey)}: %{y} ${dataset.getFeatureUnits(yAxisFeatureKey)}` + `
Track ID: %{customdata}
Object ID: %{id}` ); } From c34557a32bcc0c4484e40c29ccd39e6fb8252800 Mon Sep 17 00:00:00 2001 From: Peyton Lee Date: Fri, 22 Mar 2024 17:11:25 -0700 Subject: [PATCH 05/16] fix: Landing page feature names --- src/routes/LandingPageContent.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/routes/LandingPageContent.ts b/src/routes/LandingPageContent.ts index fab60548c..a769739e5 100644 --- a/src/routes/LandingPageContent.ts +++ b/src/routes/LandingPageContent.ts @@ -14,7 +14,7 @@ export const landingPageContent: ProjectEntry[] = [ loadParams: { collection: "https://dev-aics-dtp-001.int.allencell.org/dan-data/colorizer/data/collection.json", dataset: "Mama Bear", - feature: "Volume", + featureKey: "volume", }, inReview: true, }, @@ -29,7 +29,7 @@ export const landingPageContent: ProjectEntry[] = [ loadParams: { collection: "https://dev-aics-dtp-001.int.allencell.org/dan-data/colorizer/data/collection.json", dataset: "Mama Bear", - feature: "Height", + featureKey: "height", range: [0.54, 9.452], colorRampKey: "esri-mentone_beach", time: 154, @@ -41,7 +41,7 @@ export const landingPageContent: ProjectEntry[] = [ loadParams: { collection: "https://dev-aics-dtp-001.int.allencell.org/dan-data/colorizer/data/collection.json", dataset: "Goldilocks", - feature: "Height", + featureKey: "height", range: [1.084, 8.156], colorRampKey: "esri-blue_red_8", time: 446, @@ -53,13 +53,13 @@ export const landingPageContent: ProjectEntry[] = [ loadParams: { collection: "https://dev-aics-dtp-001.int.allencell.org/dan-data/colorizer/data/collection.json", dataset: "Baby Bear", - feature: "Height", + featureKey: "height", range: [0.54, 8.895], colorRampKey: "esri-green_brown_1", time: 397, thresholds: [ { - featureName: "Volume", + featureKey: "volume", type: ThresholdType.NUMERIC, units: "µm³", min: 649.121, From 7e25c59a92f89ea95d3ef9ecf31ac0e81fdc818a Mon Sep 17 00:00:00 2001 From: Peyton Lee Date: Fri, 22 Mar 2024 17:15:41 -0700 Subject: [PATCH 06/16] fix: Incorrect feature labels on filters --- src/colorizer/Dataset.ts | 7 +++++-- src/components/Tabs/FeatureThresholdsTab.tsx | 3 ++- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/src/colorizer/Dataset.ts b/src/colorizer/Dataset.ts index 3e09bf62c..00aa55c25 100644 --- a/src/colorizer/Dataset.ts +++ b/src/colorizer/Dataset.ts @@ -180,8 +180,8 @@ export default class Dataset { } // TODO: Throw an error if undefined? - public getFeatureName(key: string): string { - return this.features.get(key)!.name; + public getFeatureName(key: string): string | undefined { + return this.features.get(key)?.name; } /** @@ -193,6 +193,9 @@ export default class Dataset { public getFeatureNameWithUnits(key: string): string { const name = this.getFeatureName(key); + if (!name) { + return "N/A"; + } const units = this.getFeatureUnits(key); if (units) { return `${name} (${units})`; diff --git a/src/components/Tabs/FeatureThresholdsTab.tsx b/src/components/Tabs/FeatureThresholdsTab.tsx index 1fa6bf016..5089e4259 100644 --- a/src/components/Tabs/FeatureThresholdsTab.tsx +++ b/src/components/Tabs/FeatureThresholdsTab.tsx @@ -309,7 +309,8 @@ export default function FeatureThresholdsTab(inputProps: FeatureThresholdsTabPro // both in the current dataset to be enabled and editable. const featureData = props.dataset?.getFeatureData(threshold.featureKey); const disabled = featureData === undefined || featureData.units !== threshold.units; - const featureLabel = threshold.units ? `${threshold.featureKey} (${threshold.units})` : threshold.featureKey; + const name = featureData?.name || threshold.featureKey; + const featureLabel = threshold.units ? `${name} (${threshold.units})` : name; return ( From e65e494780e74b32ac2736fd0fb62d59c1c6a4bc Mon Sep 17 00:00:00 2001 From: Peyton Lee Date: Mon, 25 Mar 2024 09:52:58 -0700 Subject: [PATCH 07/16] doc: Renamed feature name references to feature keys --- src/colorizer/types.ts | 2 -- src/colorizer/utils/dataset_utils.ts | 2 +- src/colorizer/utils/url_utils.ts | 14 +++++++------- src/components/Tabs/FeatureThresholdsTab.tsx | 2 +- 4 files changed, 9 insertions(+), 11 deletions(-) diff --git a/src/colorizer/types.ts b/src/colorizer/types.ts index ac98810c3..e00320bf7 100644 --- a/src/colorizer/types.ts +++ b/src/colorizer/types.ts @@ -102,8 +102,6 @@ export enum ThresholdType { } type BaseFeatureThreshold = { - // TODO: Replace with key string - // featureKey: string; featureKey: string; units: string; type: ThresholdType; diff --git a/src/colorizer/utils/dataset_utils.ts b/src/colorizer/utils/dataset_utils.ts index 1243a52d6..f3fe05f4e 100644 --- a/src/colorizer/utils/dataset_utils.ts +++ b/src/colorizer/utils/dataset_utils.ts @@ -34,7 +34,7 @@ export type ManifestFileMetadata = Spread; // eslint-disable-next-line @typescript-eslint/naming-convention type ManifestFileV0_0_0 = { frames: string[]; - /** Map from feature name to relative path */ + /** Deprecated; Map from feature name to relative path. */ features: Record; /** Deprecated; avoid using in new datasets. Instead, use the new `FeatureMetadata` spec. */ featureMetadata?: Record< diff --git a/src/colorizer/utils/url_utils.ts b/src/colorizer/utils/url_utils.ts index 809389b84..6acb61d9c 100644 --- a/src/colorizer/utils/url_utils.ts +++ b/src/colorizer/utils/url_utils.ts @@ -104,8 +104,8 @@ export function fetchWithTimeout( * * @param threshold FeatureThreshold to serialize. * @returns A string representing the threshold. - * - For numeric features, the threshold is serialized as `featureName:unit:min:max`. - * - For categorical features, the threshold is serialized as `featureName:unit:selected_hex`, + * - For numeric features, the threshold is serialized as `featureKey:unit:min:max`. + * - For categorical features, the threshold is serialized as `featureKey:unit:selected_hex`, * where `selected_hex` is the hex form of a binary number representing what categories are selected. * * The i-th place of the binary number is `1` if the i-th category in the feature's category list is enabled. @@ -115,9 +115,9 @@ export function fetchWithTimeout( * The binary representation is `00101`, which is `0x05` in hex. */ function serializeThreshold(threshold: FeatureThreshold): string { - // featureName + units are encoded in case it contains special characters (":" or ","). + // featureKey + units are encoded in case it contains special characters (":" or ","). // TODO: remove once feature keys are implemented. - const featureName = encodeURIComponent(threshold.featureKey); + const featureKey = encodeURIComponent(threshold.featureKey); const featureUnit = encodeURIComponent(threshold.units); // TODO: Are there better characters I can be using here? ":" and "," take up @@ -129,12 +129,12 @@ function serializeThreshold(threshold: FeatureThreshold): string { selectedBinary |= (threshold.enabledCategories[i] ? 1 : 0) << i; } const selectedHex = selectedBinary.toString(16); - return `${featureName}:${featureUnit}:${selectedHex}`; + return `${featureKey}:${featureUnit}:${selectedHex}`; } else { // Numeric feature const min = numberToStringDecimal(threshold.min, 3); const max = numberToStringDecimal(threshold.max, 3); - return `${featureName}:${featureUnit}:${min}:${max}`; + return `${featureKey}:${featureUnit}:${min}:${max}`; } } @@ -151,7 +151,7 @@ function deserializeThreshold(thresholdString: string): FeatureThreshold | undef console.warn( "url_utils.deserializeThreshold: Could not parse threshold string: '" + thresholdString + - "'; feature name and/or units missing." + "'; feature key and/or units missing." ); return undefined; } diff --git a/src/components/Tabs/FeatureThresholdsTab.tsx b/src/components/Tabs/FeatureThresholdsTab.tsx index 5089e4259..38bff40fe 100644 --- a/src/components/Tabs/FeatureThresholdsTab.tsx +++ b/src/components/Tabs/FeatureThresholdsTab.tsx @@ -130,7 +130,7 @@ export default function FeatureThresholdsTab(inputProps: FeatureThresholdsTabPro const { scrollShadowStyle, onScrollHandler, scrollRef } = useScrollShadow(); const selectContainerRef = useRef(null); - /** Converts a threshold to a unique key that can be used to look up its information later. Matches on feature name and unit. */ + /** Converts a threshold to a unique key that can be used to look up its information later. Matches on feature key and unit. */ const thresholdToKey = (threshold: FeatureThreshold): string => { return `${encodeURIComponent(threshold.featureKey)}:${threshold.units ? encodeURIComponent(threshold.units) : ""}`; }; From bb6e50aa2d82a7a68bf5cf30a0a27bc5bdbe5371 Mon Sep 17 00:00:00 2001 From: Peyton Lee Date: Tue, 26 Mar 2024 11:47:29 -0700 Subject: [PATCH 08/16] refactor: Removed separate featureKey param; feature searches by key + name --- src/Viewer.tsx | 8 ++++-- src/colorizer/Dataset.ts | 1 - src/colorizer/utils/url_utils.ts | 10 +------- tests/Dataset.test.ts | 11 +++++--- tests/url_utils.test.ts | 44 ++++++++++++++++---------------- 5 files changed, 36 insertions(+), 38 deletions(-) diff --git a/src/Viewer.tsx b/src/Viewer.tsx index ae525f0cb..95852ddb3 100644 --- a/src/Viewer.tsx +++ b/src/Viewer.tsx @@ -211,7 +211,7 @@ function Viewer(): ReactElement { const state: Partial = { collection: collectionParam, dataset: datasetParam, - featureKey: featureKey, + feature: featureKey, track: selectedTrack?.trackId, // Ignore time=0 to reduce clutter time: currentFrame !== 0 ? currentFrame : undefined, @@ -497,7 +497,11 @@ function Viewer(): ReactElement { let newFeatureKey = featureKey; if (initialUrlParams.feature && dataset) { // Load feature (if unset, do nothing because replaceDataset already loads a default) - newFeatureKey = replaceFeature(dataset, initialUrlParams.feature); + const featureKeyOrName = initialUrlParams.feature; + newFeatureKey = dataset.hasFeatureKey(featureKeyOrName) + ? featureKeyOrName + : dataset.findFeatureKeyFromName(featureKeyOrName) || featureKey; // Try to look up key from name + newFeatureKey = replaceFeature(dataset, newFeatureKey); } // Range, track, and time setting must be done after the dataset and feature is set. if (initialUrlParams.range) { diff --git a/src/colorizer/Dataset.ts b/src/colorizer/Dataset.ts index 00aa55c25..cfd794629 100644 --- a/src/colorizer/Dataset.ts +++ b/src/colorizer/Dataset.ts @@ -167,7 +167,6 @@ export default class Dataset { } public findFeatureKeyFromName(name: string): string | undefined { - // TODO: Replace once you add back in featureNames return Array.from(this.features.values()).find((f) => f.name === name)?.key; } diff --git a/src/colorizer/utils/url_utils.ts b/src/colorizer/utils/url_utils.ts index 6acb61d9c..02ee9626e 100644 --- a/src/colorizer/utils/url_utils.ts +++ b/src/colorizer/utils/url_utils.ts @@ -26,9 +26,7 @@ import { numberToStringDecimal } from "./math_utils"; enum UrlParam { TRACK = "track", DATASET = "dataset", - // Will be deprecated once feature keys are implemented FEATURE = "feature", - FEATURE_KEY = "feature-key", TIME = "t", COLLECTION = "collection", THRESHOLDS = "filters", @@ -66,9 +64,8 @@ const ALLEN_PREFIX_TO_HTTPS: Record = { export type UrlParams = { collection: string; dataset: string; - /** Will be deprecated. */ + /** Either feature key or feature name. */ feature: string; - featureKey: string; track: number; time: number; thresholds: FeatureThreshold[]; @@ -395,9 +392,6 @@ export function paramsToUrlQueryString(state: Partial): string { if (state.feature) { includedParameters.push(`${UrlParam.FEATURE}=${encodeURIComponent(state.feature)}`); } - if (state.featureKey) { - includedParameters.push(`${UrlParam.FEATURE_KEY}=${encodeURIComponent(state.featureKey)}`); - } if (state.track !== undefined) { includedParameters.push(`${UrlParam.TRACK}=${state.track}`); } @@ -547,7 +541,6 @@ export function loadFromUrlSearchParams(urlParams: URLSearchParams): Partial { features: [ { key: "feature1", name: "Feature1", data: "feature1.json", units: "meters", type: "continuous" }, { key: "feature2", name: "Feature2", data: "feature2.json", units: "(m)", type: "discrete" }, - { key: "feature3", name: "Feature3", data: "feature3.json", units: "μm/s", type: "bad-type" }, - { key: "feature4", name: "Feature4", data: "feature4.json" }, + { name: "Feature3", data: "feature3.json", units: "μm/s", type: "bad-type" }, + { name: "Feature4", data: "feature4.json" }, { key: "feature5", name: "Feature5", @@ -137,8 +137,11 @@ describe("Dataset", () => { // Test both normal and deprecated manifests for (const [version, manifest] of manifestsToTest) { describe(version, () => { - it("fills in feature keys if only names are provided", () => { - throw new Error("Test not implemented"); + it("fills in feature keys if only names are provided", async () => { + const dataset = new Dataset(defaultPath, new MockFrameLoader(), new MockArrayLoader()); + const mockFetch = makeMockFetchMethod(defaultPath, manifest); + await dataset.open(mockFetch); + expect(dataset.featureKeys).to.deep.equal(["feature1", "feature2", "feature3", "feature4", "feature5"]); }); it("retrieves feature units", async () => { diff --git a/tests/url_utils.test.ts b/tests/url_utils.test.ts index 16782b62d..60e4eccf1 100644 --- a/tests/url_utils.test.ts +++ b/tests/url_utils.test.ts @@ -125,18 +125,18 @@ describe("Loading + saving from URL query strings", () => { track: 25, time: 14, thresholds: [ - { featureName: "f1", units: "m", type: ThresholdType.NUMERIC, min: 0, max: 0 }, - { featureName: "f2", units: "um", type: ThresholdType.NUMERIC, min: NaN, max: NaN }, - { featureName: "f3", units: "km", type: ThresholdType.NUMERIC, min: 0, max: 1 }, - { featureName: "f4", units: "mm", type: ThresholdType.NUMERIC, min: 0.501, max: 1000.485 }, + { featureKey: "f1", units: "m", type: ThresholdType.NUMERIC, min: 0, max: 0 }, + { featureKey: "f2", units: "um", type: ThresholdType.NUMERIC, min: NaN, max: NaN }, + { featureKey: "f3", units: "km", type: ThresholdType.NUMERIC, min: 0, max: 1 }, + { featureKey: "f4", units: "mm", type: ThresholdType.NUMERIC, min: 0.501, max: 1000.485 }, { - featureName: "f5", + featureKey: "f5", units: "", type: ThresholdType.CATEGORICAL, enabledCategories: [true, true, true, true, true, true, true, true, true, true, true, true], }, { - featureName: "f6", + featureKey: "f6", units: "", type: ThresholdType.CATEGORICAL, enabledCategories: [true, false, false, false, true, false, false, false, false, false, false, false], @@ -177,11 +177,11 @@ describe("Loading + saving from URL query strings", () => { it("Handles feature threshold names with nonstandard characters", () => { const originalParams: Partial = { thresholds: [ - { featureName: "feature,,,", units: ",m,", type: ThresholdType.NUMERIC, min: 0, max: 1 }, - { featureName: "(feature)", units: "(m)", type: ThresholdType.NUMERIC, min: 0, max: 1 }, - { featureName: "feat:ure", units: ":m", type: ThresholdType.NUMERIC, min: 0, max: 1 }, + { featureKey: "feature,,,", units: ",m,", type: ThresholdType.NUMERIC, min: 0, max: 1 }, + { featureKey: "(feature)", units: "(m)", type: ThresholdType.NUMERIC, min: 0, max: 1 }, + { featureKey: "feat:ure", units: ":m", type: ThresholdType.NUMERIC, min: 0, max: 1 }, { - featureName: "0.0%", + featureKey: "0.0%", units: "m&m's", type: ThresholdType.CATEGORICAL, enabledCategories: padCategories([true, false, false]), @@ -200,9 +200,9 @@ describe("Loading + saving from URL query strings", () => { it("Enforces min/max on range and thresholds", () => { const originalParams: Partial = { thresholds: [ - { featureName: "feature1", units: "m", type: ThresholdType.NUMERIC, min: 1, max: 0 }, - { featureName: "feature2", units: "m", type: ThresholdType.NUMERIC, min: 12, max: -34 }, - { featureName: "feature3", units: "m", type: ThresholdType.NUMERIC, min: 0.5, max: 0.25 }, + { featureKey: "feature1", units: "m", type: ThresholdType.NUMERIC, min: 1, max: 0 }, + { featureKey: "feature2", units: "m", type: ThresholdType.NUMERIC, min: 12, max: -34 }, + { featureKey: "feature3", units: "m", type: ThresholdType.NUMERIC, min: 0.5, max: 0.25 }, ], range: [1, 0], }; @@ -214,16 +214,16 @@ describe("Loading + saving from URL query strings", () => { const parsedParams = loadFromUrlSearchParams(new URLSearchParams(queryString)); expect(parsedParams.thresholds).deep.equals([ - { featureName: "feature1", units: "m", type: ThresholdType.NUMERIC, min: 0, max: 1 }, - { featureName: "feature2", units: "m", type: ThresholdType.NUMERIC, min: -34, max: 12 }, - { featureName: "feature3", units: "m", type: ThresholdType.NUMERIC, min: 0.25, max: 0.5 }, + { featureKey: "feature1", units: "m", type: ThresholdType.NUMERIC, min: 0, max: 1 }, + { featureKey: "feature2", units: "m", type: ThresholdType.NUMERIC, min: -34, max: 12 }, + { featureKey: "feature3", units: "m", type: ThresholdType.NUMERIC, min: 0.25, max: 0.5 }, ]); expect(parsedParams.range).deep.equals([0, 1]); }); it("Handles empty feature thresholds", () => { const originalParams: Partial = { - thresholds: [{ featureName: "feature1", units: "", type: ThresholdType.NUMERIC, min: 0, max: 1 }], + thresholds: [{ featureKey: "feature1", units: "", type: ThresholdType.NUMERIC, min: 0, max: 1 }], }; const queryString = paramsToUrlQueryString(originalParams); const expectedQueryString = "?filters=feature1%3A%3A0%3A1"; @@ -238,7 +238,7 @@ describe("Loading + saving from URL query strings", () => { time: 0, track: 0, range: [0, 0], - thresholds: [{ featureName: "feature", units: "", type: ThresholdType.NUMERIC, min: 0, max: 0 }], + thresholds: [{ featureKey: "feature", units: "", type: ThresholdType.NUMERIC, min: 0, max: 0 }], }; const queryString = paramsToUrlQueryString(originalParams); const expectedQueryString = "?track=0&t=0&filters=feature%3A%3A0%3A0&range=0%2C0"; @@ -250,7 +250,7 @@ describe("Loading + saving from URL query strings", () => { it("Handles less than the maximum expected categories", () => { const originalParams: Partial = { - thresholds: [{ featureName: "feature", units: "", type: ThresholdType.CATEGORICAL, enabledCategories: [true] }], + thresholds: [{ featureKey: "feature", units: "", type: ThresholdType.CATEGORICAL, enabledCategories: [true] }], }; const queryString = paramsToUrlQueryString(originalParams); const expectedQueryString = "?filters=feature%3A%3A1"; @@ -259,7 +259,7 @@ describe("Loading + saving from URL query strings", () => { const parsedParams = loadFromUrlSearchParams(new URLSearchParams(queryString)); expect(parsedParams.thresholds).deep.equals([ { - featureName: "feature", + featureKey: "feature", units: "", type: ThresholdType.CATEGORICAL, enabledCategories: padCategories([true]), @@ -272,7 +272,7 @@ describe("Loading + saving from URL query strings", () => { thresholds.push(true); // Add an extra threshold. This should be ignored const originalParams: Partial = { thresholds: [ - { featureName: "feature", units: "", type: ThresholdType.CATEGORICAL, enabledCategories: thresholds }, + { featureKey: "feature", units: "", type: ThresholdType.CATEGORICAL, enabledCategories: thresholds }, ], }; const queryString = paramsToUrlQueryString(originalParams); @@ -282,7 +282,7 @@ describe("Loading + saving from URL query strings", () => { const parsedParams = loadFromUrlSearchParams(new URLSearchParams(queryString)); expect(parsedParams.thresholds).deep.equals([ { - featureName: "feature", + featureKey: "feature", units: "", type: ThresholdType.CATEGORICAL, enabledCategories: padCategories([true, true]), From 3d14688d73e00db9e629a508451d00ef928b1e4f Mon Sep 17 00:00:00 2001 From: Peyton Lee Date: Tue, 26 Mar 2024 12:08:39 -0700 Subject: [PATCH 09/16] feat: Added convenience method for looking up feature key/name --- src/Viewer.tsx | 6 +----- src/colorizer/Dataset.ts | 18 ++++++++++++++++++ src/colorizer/utils/data_utils.ts | 19 ++++++++++++++++--- 3 files changed, 35 insertions(+), 8 deletions(-) diff --git a/src/Viewer.tsx b/src/Viewer.tsx index 95852ddb3..02b4e85e7 100644 --- a/src/Viewer.tsx +++ b/src/Viewer.tsx @@ -497,11 +497,7 @@ function Viewer(): ReactElement { let newFeatureKey = featureKey; if (initialUrlParams.feature && dataset) { // Load feature (if unset, do nothing because replaceDataset already loads a default) - const featureKeyOrName = initialUrlParams.feature; - newFeatureKey = dataset.hasFeatureKey(featureKeyOrName) - ? featureKeyOrName - : dataset.findFeatureKeyFromName(featureKeyOrName) || featureKey; // Try to look up key from name - newFeatureKey = replaceFeature(dataset, newFeatureKey); + newFeatureKey = replaceFeature(dataset, dataset.findFeatureByKeyOrName(initialUrlParams.feature) || featureKey); } // Range, track, and time setting must be done after the dataset and feature is set. if (initialUrlParams.range) { diff --git a/src/colorizer/Dataset.ts b/src/colorizer/Dataset.ts index cfd794629..fc8757902 100644 --- a/src/colorizer/Dataset.ts +++ b/src/colorizer/Dataset.ts @@ -166,6 +166,24 @@ export default class Dataset { return this.featureKeys.includes(key); } + /** + * Returns the feature key if a feature with a matching key or name exists in the + * dataset. + * @param keyOrName String key or name of the feature to find. + * @returns The feature key if found, otherwise undefined. + */ + public findFeatureByKeyOrName(keyOrName: string): string | undefined { + if (this.hasFeatureKey(keyOrName)) { + return keyOrName; + } else { + return this.findFeatureKeyFromName(keyOrName); + } + } + + /** + * Attempts to find a matching feature key for a feature name. + * @returns The feature key if found, otherwise undefined. + */ public findFeatureKeyFromName(name: string): string | undefined { return Array.from(this.features.values()).find((f) => f.name === name)?.key; } diff --git a/src/colorizer/utils/data_utils.ts b/src/colorizer/utils/data_utils.ts index 07a6ca6fd..d89cbb498 100644 --- a/src/colorizer/utils/data_utils.ts +++ b/src/colorizer/utils/data_utils.ts @@ -45,9 +45,22 @@ export function validateThresholds(dataset: Dataset, thresholds: FeatureThreshol const newThresholds: FeatureThreshold[] = []; for (const threshold of thresholds) { - const featureData = dataset.getFeatureData(threshold.featureKey); + // `featureKey` may be a name for backwards-compatibility. Convert it to the key if it exists in the dataset. + // This will still match against the units below + let featureKey = threshold.featureKey; + const matchedFeatureKey = dataset.findFeatureByKeyOrName(threshold.featureKey); + if (matchedFeatureKey !== undefined) { + featureKey = matchedFeatureKey; + } + + const featureData = dataset.getFeatureData(featureKey); const isInDataset = featureData && featureData.units === threshold.units; + if (isInDataset) { + // Update feature key just in case it was a name + threshold.featureKey = featureKey; + } + if (isInDataset && featureData.type === FeatureType.CATEGORICAL && isThresholdNumeric(threshold)) { // Threshold is not categorical but the feature is. // Convert the threshold to categorical. @@ -55,7 +68,7 @@ export function validateThresholds(dataset: Dataset, thresholds: FeatureThreshol // This is important for historical reasons, because older versions of the app used to only store features as numeric // thresholds. This would cause categorical features loaded from the URL to be incorrectly shown on the UI. newThresholds.push({ - featureKey: threshold.featureKey, + featureKey: featureKey, units: threshold.units, type: ThresholdType.CATEGORICAL, enabledCategories: Array(MAX_FEATURE_CATEGORIES).fill(true), @@ -64,7 +77,7 @@ export function validateThresholds(dataset: Dataset, thresholds: FeatureThreshol // Threshold is categorical but the feature is not. // Convert to numeric threshold instead. newThresholds.push({ - featureKey: threshold.featureKey, + featureKey: featureKey, units: threshold.units, type: ThresholdType.NUMERIC, min: featureData.min, From 6ede5b2f6272de6e3cc710f1c5a75627ea8f132f Mon Sep 17 00:00:00 2001 From: Peyton Lee Date: Tue, 26 Mar 2024 12:09:06 -0700 Subject: [PATCH 10/16] refactor: Made `makeMockDataset` to simplify Dataset tests --- tests/Dataset.test.ts | 137 ++++++++++++++++++++---------------------- 1 file changed, 65 insertions(+), 72 deletions(-) diff --git a/tests/Dataset.test.ts b/tests/Dataset.test.ts index 0f9efa526..bc7db79bc 100644 --- a/tests/Dataset.test.ts +++ b/tests/Dataset.test.ts @@ -10,61 +10,68 @@ import { ANY_ERROR } from "./test_utils"; import Dataset, { FeatureType } from "../src/colorizer/Dataset"; -describe("Dataset", () => { - const defaultPath = "https://some-path.json"; - - const makeMockFetchMethod = (url: string, manifestJson: T): ((url: string) => Promise) => { - return async (inputUrl: string): Promise => { - if (inputUrl !== url) { - throw new Error(`Input url '${inputUrl}' does not match expected url '${url}'.`); - } - return Promise.resolve(manifestJson); - }; +const DEFAULT_PATH = "https://some-path.json"; + +const makeMockFetchMethod = (url: string, manifestJson: T): ((url: string) => Promise) => { + return async (inputUrl: string): Promise => { + if (inputUrl !== url) { + throw new Error(`Input url '${inputUrl}' does not match expected url '${url}'.`); + } + return Promise.resolve(manifestJson); }; +}; - class MockFrameLoader implements IFrameLoader { - width: number; - height: number; +class MockFrameLoader implements IFrameLoader { + width: number; + height: number; - constructor(width: number = 1, height: number = 1) { - this.width = width; - this.height = height; - } + constructor(width: number = 1, height: number = 1) { + this.width = width; + this.height = height; + } - load(_url: string): Promise { - return Promise.resolve( - new DataTexture( - new Uint8Array(this.width * this.height * 4), - this.width, - this.height, - RGBAFormat, - UnsignedByteType - ) - ); - } + load(_url: string): Promise { + return Promise.resolve( + new DataTexture( + new Uint8Array(this.width * this.height * 4), + this.width, + this.height, + RGBAFormat, + UnsignedByteType + ) + ); } +} - class MockArraySource implements ArraySource { - getBuffer(type: T): FeatureArrayType[T] { - return new featureTypeSpecs[type].ArrayConstructor([]); - } - getTexture(_type: FeatureDataType): Texture { - return new Texture(); - } - getMin(): number { - return 0; - } - getMax(): number { - return 1; - } +class MockArraySource implements ArraySource { + getBuffer(type: T): FeatureArrayType[T] { + return new featureTypeSpecs[type].ArrayConstructor([]); + } + getTexture(_type: FeatureDataType): Texture { + return new Texture(); + } + getMin(): number { + return 0; } + getMax(): number { + return 1; + } +} - class MockArrayLoader implements IArrayLoader { - load(_url: string): Promise { - return Promise.resolve(new MockArraySource()); - } +class MockArrayLoader implements IArrayLoader { + load(_url: string): Promise { + return Promise.resolve(new MockArraySource()); } +} +export const makeMockDataset = async (manifest: AnyManifestFile): Promise => { + const dataset = new Dataset(DEFAULT_PATH, new MockFrameLoader(), new MockArrayLoader()); + const mockFetch = makeMockFetchMethod(DEFAULT_PATH, manifest); + await dataset.open(mockFetch); + return dataset; +}; + +describe("Dataset", () => { // eslint-disable-next-line @typescript-eslint/naming-convention const manifestV0_0_0: AnyManifestFile = { frames: ["frame0.json"], @@ -138,16 +145,12 @@ describe("Dataset", () => { for (const [version, manifest] of manifestsToTest) { describe(version, () => { it("fills in feature keys if only names are provided", async () => { - const dataset = new Dataset(defaultPath, new MockFrameLoader(), new MockArrayLoader()); - const mockFetch = makeMockFetchMethod(defaultPath, manifest); - await dataset.open(mockFetch); + const dataset = await makeMockDataset(manifest); expect(dataset.featureKeys).to.deep.equal(["feature1", "feature2", "feature3", "feature4", "feature5"]); }); it("retrieves feature units", async () => { - const dataset = new Dataset(defaultPath, new MockFrameLoader(), new MockArrayLoader()); - const mockFetch = makeMockFetchMethod(defaultPath, manifest); - await dataset.open(mockFetch); + const dataset = await makeMockDataset(manifest); expect(dataset.getFeatureUnits("feature1")).to.equal("meters"); expect(dataset.getFeatureUnits("feature2")).to.equal("(m)"); @@ -157,9 +160,7 @@ describe("Dataset", () => { }); it("retrieves feature types", async () => { - const dataset = new Dataset(defaultPath, new MockFrameLoader(), new MockArrayLoader()); - const mockFetch = makeMockFetchMethod(defaultPath, manifest); - await dataset.open(mockFetch); + const dataset = await makeMockDataset(manifest); expect(dataset.getFeatureType("feature1")).to.equal(FeatureType.CONTINUOUS); expect(dataset.getFeatureType("feature2")).to.equal(FeatureType.DISCRETE); @@ -167,18 +168,14 @@ describe("Dataset", () => { }); it("defaults type to continuous if no type or bad type provided", async () => { - const dataset = new Dataset(defaultPath, new MockFrameLoader(), new MockArrayLoader()); - const mockFetch = makeMockFetchMethod(defaultPath, manifest); - await dataset.open(mockFetch); + const dataset = await makeMockDataset(manifest); expect(dataset.getFeatureType("feature3")).to.equal(FeatureType.CONTINUOUS); expect(dataset.getFeatureType("feature4")).to.equal(FeatureType.CONTINUOUS); }); it("gets whether features are categorical", async () => { - const dataset = new Dataset(defaultPath, new MockFrameLoader(), new MockArrayLoader()); - const mockFetch = makeMockFetchMethod(defaultPath, manifest); - await dataset.open(mockFetch); + const dataset = await makeMockDataset(manifest); expect(dataset.isFeatureCategorical("feature1")).to.be.false; expect(dataset.isFeatureCategorical("feature2")).to.be.false; @@ -188,9 +185,7 @@ describe("Dataset", () => { }); it("gets feature categories", async () => { - const dataset = new Dataset(defaultPath, new MockFrameLoader(), new MockArrayLoader()); - const mockFetch = makeMockFetchMethod(defaultPath, manifest); - await dataset.open(mockFetch); + const dataset = await makeMockDataset(manifest); expect(dataset.getFeatureCategories("feature1")).to.deep.equal(null); expect(dataset.getFeatureCategories("feature2")).to.deep.equal(null); @@ -209,8 +204,8 @@ describe("Dataset", () => { feature1: { type: "categorical" }, }, }; - const dataset = new Dataset(defaultPath, new MockFrameLoader(), new MockArrayLoader()); - const mockFetch = makeMockFetchMethod(defaultPath, badManifest); + const dataset = new Dataset(DEFAULT_PATH, new MockFrameLoader(), new MockArrayLoader()); + const mockFetch = makeMockFetchMethod(DEFAULT_PATH, badManifest); await expect(dataset.open(mockFetch)).rejects.toThrowError(ANY_ERROR); }); @@ -228,13 +223,13 @@ describe("Dataset", () => { }, }, }; - const dataset = new Dataset(defaultPath, new MockFrameLoader(), new MockArrayLoader()); - const mockFetch = makeMockFetchMethod(defaultPath, badManifest); + const dataset = new Dataset(DEFAULT_PATH, new MockFrameLoader(), new MockArrayLoader()); + const mockFetch = makeMockFetchMethod(DEFAULT_PATH, badManifest); await expect(dataset.open(mockFetch)).rejects.toThrowError(ANY_ERROR); }); it("Loads the first frame and retrieves frame dimensions on open", async () => { - const mockFetch = makeMockFetchMethod(defaultPath, manifest); + const mockFetch = makeMockFetchMethod(DEFAULT_PATH, manifest); const dimensionTests = [ [0, 0], [1, 1], @@ -246,7 +241,7 @@ describe("Dataset", () => { ]; for (const [width, height] of dimensionTests) { - const dataset = new Dataset(defaultPath, new MockFrameLoader(width, height), new MockArrayLoader()); + const dataset = new Dataset(DEFAULT_PATH, new MockFrameLoader(width, height), new MockArrayLoader()); expect(dataset.frameResolution).to.deep.equal(new Vector2(1, 1)); await dataset.open(mockFetch); expect(dataset.frameResolution).to.deep.equal(new Vector2(width, height)); @@ -254,9 +249,7 @@ describe("Dataset", () => { }); it("Loads metadata", async () => { - const dataset = new Dataset(defaultPath, new MockFrameLoader(), new MockArrayLoader()); - const mockFetch = makeMockFetchMethod(defaultPath, manifest); - await dataset.open(mockFetch); + const dataset = await makeMockDataset(manifest); // Default metadata should be auto-filled with default values if (semver.lt(version, "1.0.0")) { From d8c56f43c9fdfbec2aa78959689c1a7404b3ac40 Mon Sep 17 00:00:00 2001 From: Peyton Lee Date: Tue, 26 Mar 2024 12:17:07 -0700 Subject: [PATCH 11/16] feat: Added unit test for `validateThresholds` updating to keys --- tests/data_utils.test.ts | 74 +++++++++++++++++++++++++++++++++++++++- 1 file changed, 73 insertions(+), 1 deletion(-) diff --git a/tests/data_utils.test.ts b/tests/data_utils.test.ts index 277801c42..03027b814 100644 --- a/tests/data_utils.test.ts +++ b/tests/data_utils.test.ts @@ -1,6 +1,9 @@ import { describe, expect, it } from "vitest"; -import { getKeyFromName } from "../src/colorizer/utils/data_utils"; +import { FeatureThreshold, ThresholdType } from "../src/colorizer/types"; +import { getKeyFromName, validateThresholds } from "../src/colorizer/utils/data_utils"; + +import { makeMockDataset } from "./Dataset.test"; describe("data_utils", () => { describe("getKeyFromName", () => { @@ -29,4 +32,73 @@ describe("data_utils", () => { expect(getKeyFromName("&Another Name%")).toBe("_another_name_"); }); }); + + describe("validateThresholds", () => { + it("replaces feature names with keys", async () => { + const dataset = await makeMockDataset({ + frames: ["frame0.json"], + features: [ + { name: "Feature A", key: "feature_a", data: "feature1.json", units: "", type: "discrete" }, + { name: "MY FEATURE B", key: "feature_b", data: "feature2.json", units: "", type: "discrete" }, + { + name: "My Feature C", + key: "feature_c", + data: "feature3.json", + units: "b", + type: "continuous", + categories: ["1", "2", "3"], + }, + ], + }); + + const existingThresholds: FeatureThreshold[] = [ + { + featureKey: "Feature A", + units: "", + type: ThresholdType.NUMERIC, + min: 0, + max: 10, + }, + { + featureKey: "MY FEATURE B", + units: "", + type: ThresholdType.NUMERIC, + min: 0, + max: 10, + }, + { + featureKey: "My Feature C", + units: "will_not_match_unit", + type: ThresholdType.CATEGORICAL, + enabledCategories: [true, false, false], + }, + ]; + + const newThresholds = validateThresholds(dataset, existingThresholds); + + expect(newThresholds).to.deep.equal([ + { + featureKey: "feature_a", + units: "", + type: ThresholdType.NUMERIC, + min: 0, + max: 10, + }, + { + featureKey: "feature_b", + units: "", + type: ThresholdType.NUMERIC, + min: 0, + max: 10, + }, + // Ignores features that don't match the units of the dataset's feature + { + featureKey: "My Feature C", + units: "will_not_match_unit", + type: ThresholdType.CATEGORICAL, + enabledCategories: [true, false, false], + }, + ]); + }); + }); }); From 900326f93c23af7b6264b83549ed5fdc34ded26d Mon Sep 17 00:00:00 2001 From: Peyton Lee Date: Tue, 26 Mar 2024 12:18:37 -0700 Subject: [PATCH 12/16] fix: Linting --- src/routes/LandingPageContent.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/routes/LandingPageContent.ts b/src/routes/LandingPageContent.ts index a769739e5..c54ad1b9a 100644 --- a/src/routes/LandingPageContent.ts +++ b/src/routes/LandingPageContent.ts @@ -14,7 +14,7 @@ export const landingPageContent: ProjectEntry[] = [ loadParams: { collection: "https://dev-aics-dtp-001.int.allencell.org/dan-data/colorizer/data/collection.json", dataset: "Mama Bear", - featureKey: "volume", + feature: "volume", }, inReview: true, }, @@ -29,7 +29,7 @@ export const landingPageContent: ProjectEntry[] = [ loadParams: { collection: "https://dev-aics-dtp-001.int.allencell.org/dan-data/colorizer/data/collection.json", dataset: "Mama Bear", - featureKey: "height", + feature: "height", range: [0.54, 9.452], colorRampKey: "esri-mentone_beach", time: 154, @@ -41,7 +41,7 @@ export const landingPageContent: ProjectEntry[] = [ loadParams: { collection: "https://dev-aics-dtp-001.int.allencell.org/dan-data/colorizer/data/collection.json", dataset: "Goldilocks", - featureKey: "height", + feature: "height", range: [1.084, 8.156], colorRampKey: "esri-blue_red_8", time: 446, @@ -53,7 +53,7 @@ export const landingPageContent: ProjectEntry[] = [ loadParams: { collection: "https://dev-aics-dtp-001.int.allencell.org/dan-data/colorizer/data/collection.json", dataset: "Baby Bear", - featureKey: "height", + feature: "height", range: [0.54, 8.895], colorRampKey: "esri-green_brown_1", time: 397, From a91379aee59312d90a28007731b0540a0e516c0e Mon Sep 17 00:00:00 2001 From: Peyton Lee Date: Tue, 26 Mar 2024 13:33:38 -0700 Subject: [PATCH 13/16] refactor: Code cleanup in Dataset, removed comments --- src/colorizer/Dataset.ts | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/src/colorizer/Dataset.ts b/src/colorizer/Dataset.ts index fc8757902..bbce23e0d 100644 --- a/src/colorizer/Dataset.ts +++ b/src/colorizer/Dataset.ts @@ -60,7 +60,7 @@ export default class Dataset { private arrayLoader: IArrayLoader; // Use map to enforce ordering - /** Maps, in order, from feature keys to feature data. */ + /** Ordered map from feature keys to feature data. */ private features: Map; private outlierFile?: string; @@ -196,7 +196,6 @@ export default class Dataset { return this.features.get(key); } - // TODO: Throw an error if undefined? public getFeatureName(key: string): string | undefined { return this.features.get(key)?.name; } @@ -288,10 +287,6 @@ export default class Dataset { return Array.from(this.features.keys()); } - // public get featureNames(): string[] { - // return Array.from(this.features.values()).map((f) => f.name); - // } - public get numObjects(): number { const featureData = this.getFeatureData(this.featureKeys[0]); if (!featureData) { From 427fa657ed7f899e6d0b8e2205ae03d7837593fc Mon Sep 17 00:00:00 2001 From: Peyton Lee Date: Tue, 26 Mar 2024 14:04:38 -0700 Subject: [PATCH 14/16] refactor: Code cleanup --- src/colorizer/utils/data_utils.ts | 13 +++++++++---- src/colorizer/utils/url_utils.ts | 1 - src/components/Tabs/FeatureThresholdsTab.tsx | 2 ++ tests/data_utils.test.ts | 6 ++++-- 4 files changed, 15 insertions(+), 7 deletions(-) diff --git a/src/colorizer/utils/data_utils.ts b/src/colorizer/utils/data_utils.ts index d89cbb498..5a138026b 100644 --- a/src/colorizer/utils/data_utils.ts +++ b/src/colorizer/utils/data_utils.ts @@ -34,7 +34,8 @@ export function getColorMap(colorRampData: Map, key: stri /** * Validates the thresholds against the dataset. If the threshold's feature is present but the wrong type, it is updated. * This is most likely to happen when datasets have different types for the same feature key, or if thresholds are loaded from - * an outdated URL. + * an outdated URL. Also changes feature names to keys if they are present in the dataset for backwards-compatibility. + * * @param dataset The dataset to validate thresholds against. * @param thresholds An array of `FeatureThresholds` to validate. * @returns a new array of thresholds, with any categorical thresholds converted to numeric thresholds if the feature is numeric @@ -45,8 +46,8 @@ export function validateThresholds(dataset: Dataset, thresholds: FeatureThreshol const newThresholds: FeatureThreshold[] = []; for (const threshold of thresholds) { - // `featureKey` may be a name for backwards-compatibility. Convert it to the key if it exists in the dataset. - // This will still match against the units below + // Under the old URL schema, `featureKey` may be a name. Convert it to a key if a matching feature exists in the dataset. + // Note that units will also need to match for the threshold to be valid for this dataset. let featureKey = threshold.featureKey; const matchedFeatureKey = dataset.findFeatureByKeyOrName(threshold.featureKey); if (matchedFeatureKey !== undefined) { @@ -57,7 +58,7 @@ export function validateThresholds(dataset: Dataset, thresholds: FeatureThreshol const isInDataset = featureData && featureData.units === threshold.units; if (isInDataset) { - // Update feature key just in case it was a name + // Threshold key + unit matches, so update feature key just in case it was a name threshold.featureKey = featureKey; } @@ -135,6 +136,10 @@ export function getInRangeLUT(dataset: Dataset, thresholds: FeatureThreshold[]): return inRangeIds; } +/** + * Sanitizes a string name to a key for internal use. Replaces all non-alphanumeric characters with underscores, + * and converts the string to lowercase. + */ export function getKeyFromName(name: string): string { return name.toLowerCase().replaceAll(/[^a-z0-9_]/g, "_"); } diff --git a/src/colorizer/utils/url_utils.ts b/src/colorizer/utils/url_utils.ts index 02ee9626e..68bda4f7d 100644 --- a/src/colorizer/utils/url_utils.ts +++ b/src/colorizer/utils/url_utils.ts @@ -366,7 +366,6 @@ function deserializeScatterPlotConfig(params: URLSearchParams): Partial { describe("validateThresholds", () => { it("replaces feature names with keys", async () => { + // For backwards-compatibility, feature keys in thresholds can sometimes be feature names. These should + // be detected if they match features in the dataset, and replaced with their corresonding feature keys. const dataset = await makeMockDataset({ frames: ["frame0.json"], features: [ @@ -68,7 +70,7 @@ describe("data_utils", () => { }, { featureKey: "My Feature C", - units: "will_not_match_unit", + units: "different_unit_and_wont_match", type: ThresholdType.CATEGORICAL, enabledCategories: [true, false, false], }, @@ -94,7 +96,7 @@ describe("data_utils", () => { // Ignores features that don't match the units of the dataset's feature { featureKey: "My Feature C", - units: "will_not_match_unit", + units: "different_unit_and_wont_match", type: ThresholdType.CATEGORICAL, enabledCategories: [true, false, false], }, From cf37af6ac6d39415cfec2bb4aad868003f90fd18 Mon Sep 17 00:00:00 2001 From: Peyton Lee Date: Tue, 26 Mar 2024 14:10:41 -0700 Subject: [PATCH 15/16] fix: Handle scatter plot axes being names instead of keys --- src/Viewer.tsx | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/Viewer.tsx b/src/Viewer.tsx index 02b4e85e7..507a96bcc 100644 --- a/src/Viewer.tsx +++ b/src/Viewer.tsx @@ -530,6 +530,14 @@ function Viewer(): ReactElement { updateConfig(initialUrlParams.config); } if (initialUrlParams.scatterPlotConfig) { + const newScatterPlotConfig = initialUrlParams.scatterPlotConfig; + // For backwards-compatibility, cast xAxis and yAxis to feature keys. + if (newScatterPlotConfig.xAxis) { + newScatterPlotConfig.xAxis = dataset?.findFeatureByKeyOrName(newScatterPlotConfig.xAxis); + } + if (newScatterPlotConfig.yAxis) { + newScatterPlotConfig.yAxis = dataset?.findFeatureByKeyOrName(newScatterPlotConfig.yAxis); + } updateScatterPlotConfig(initialUrlParams.scatterPlotConfig); } }; From d0dfe32aeb632663973e87df3cf63f2fdd84513d Mon Sep 17 00:00:00 2001 From: Peyton Lee Date: Fri, 29 Mar 2024 09:19:21 -0700 Subject: [PATCH 16/16] refactor: Removed redundant `getKeyFromName` in `dataset_utils`. --- src/colorizer/utils/dataset_utils.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/colorizer/utils/dataset_utils.ts b/src/colorizer/utils/dataset_utils.ts index f3fe05f4e..44c111367 100644 --- a/src/colorizer/utils/dataset_utils.ts +++ b/src/colorizer/utils/dataset_utils.ts @@ -1,6 +1,5 @@ // Defines types for working with dataset manifests, and methods for // updating manifests from one version to another. -import { getKeyFromName } from "./data_utils"; import { Spread } from "./type_utils"; // eslint-disable-next-line @typescript-eslint/naming-convention @@ -106,7 +105,6 @@ export const updateManifestVersion = (manifest: AnyManifestFile): ManifestFile = const featureMetadata = manifest.featureMetadata?.[featureName]; features.push({ name: featureName, - key: getKeyFromName(featureName), data: featurePath, units: featureMetadata?.units || undefined, type: featureMetadata?.type || undefined,