Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix support for enum/bool auxiliary signals in NX Line visualization #1738

Merged
merged 1 commit into from
Jan 23, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion packages/app/src/providers/mock/mock-file.ts
Original file line number Diff line number Diff line change
Expand Up @@ -284,11 +284,21 @@ export function makeMockFile(): GroupWithChildren {
}),
nxGroup('numeric-like', 'NXprocess', {
children: [
nxData('bool', { signal: array('twoD_bool') }),
nxData('bool', {
signal: array('twoD_bool'),
auxiliary: { secondary_bool: array('secondary_bool') },
auxAttr: ['secondary_bool'],
}),
nxData('enum', {
signal: array('twoD_enum', {
type: enumType(intType(false, 8), ENUM_MAPPING),
}),
auxiliary: {
secondary_enum: array('secondary_enum', {
type: enumType(intType(false, 8), ENUM_MAPPING),
}),
},
auxAttr: ['secondary_enum'],
}),
],
}),
Expand Down
23 changes: 15 additions & 8 deletions packages/app/src/vis-packs/core/hooks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import {
type ArrayShape,
type ArrayValue,
type Dataset,
type NumericLikeType,
type ScalarShape,
type Value,
} from '@h5web/shared/hdf5-models';
Expand All @@ -25,6 +26,12 @@ import {

export const useToNumArray = createMemo(toNumArray);

export function useToNumArrays(
arrays: ArrayValue<NumericLikeType>[],
): NumArray[] {
return useMemo(() => arrays.map(toNumArray), arrays); // eslint-disable-line react-hooks/exhaustive-deps
}

export function useValuesInCache(
...datasets: (Dataset<ScalarShape | ArrayShape> | undefined)[]
): (dimMapping: DimensionMapping) => boolean {
Expand Down Expand Up @@ -129,23 +136,23 @@ export function useMappedArray(
return useMemo(() => applyMapping(baseArray, mapping), [baseArray, mapping]);
}

export function useMappedArrays(
values: NumArray[],
export function useMappedArrays<T extends ArrayValue>(
values: T[],
dims: number[],
mapping: DimensionMapping,
): NdArray<NumArray>[];
): NdArray<T>[];

export function useMappedArrays(
values: (NumArray | undefined)[],
export function useMappedArrays<T extends ArrayValue>(
values: (T | undefined)[],
dims: number[],
mapping: DimensionMapping,
): (NdArray<NumArray> | undefined)[];
): (NdArray<T> | undefined)[];

export function useMappedArrays(
values: (NumArray | undefined)[],
values: (ArrayValue | undefined)[],
dims: number[],
mapping: DimensionMapping,
): (NdArray<NumArray> | undefined)[] {
): (NdArray<ArrayValue> | undefined)[] {
const baseArrays = useMemo(
() => values.map((arr) => getBaseArray(arr, dims)),
[dims, ...values], // eslint-disable-line react-hooks/exhaustive-deps
Expand Down
15 changes: 8 additions & 7 deletions packages/app/src/vis-packs/core/line/MappedLineVis.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@ import {
} from '@h5web/lib';
import {
type ArrayShape,
type ArrayValue,
type Dataset,
type NumericLikeType,
type Value,
} from '@h5web/shared/hdf5-models';
import { type AxisMapping } from '@h5web/shared/nexus-models';
import { type NumArray } from '@h5web/shared/vis-models';
Expand All @@ -24,18 +24,19 @@ import {
useMappedArrays,
useSlicedDimsAndMapping,
useToNumArray,
useToNumArrays,
} from '../hooks';
import { DEFAULT_DOMAIN, formatNumLikeType, getSliceSelection } from '../utils';
import { type LineConfig } from './config';
import LineToolbar from './LineToolbar';

interface Props {
dataset?: Dataset<ArrayShape, NumericLikeType>;
value: ArrayValue<NumericLikeType>;
dataset: Dataset<ArrayShape, NumericLikeType>;
value: Value<Props['dataset']>;
valueLabel?: string;
errors?: NumArray;
auxLabels?: string[];
auxValues?: NumArray[];
auxValues?: Value<Props['dataset']>[];
Copy link
Member

@loichuder loichuder Jan 22, 2025

Choose a reason for hiding this comment

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

I wonder if it might not be clearer to introduce a new type alias here to not reference dataset explicitely.

It seems weird since auxValues are not stored in dataset. It just so happens that auxiliary dataset values have the same type as the dataset values.

Copy link
Contributor Author

@axelboc axelboc Jan 23, 2025

Choose a reason for hiding this comment

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

Yeah, wasn't sure that it was clearer. Two goals here:

  1. The types of value and of each item in auxValues should match (if an NXdata group has a boolean signal, then it should be allowed to have boolean auxiliaries). This requires changing auxValues?: NumArray[] at least to ArrayValue<NumericLikeType> to match the value prop.
  2. The dtype passed to Dataset<ArrayShape, DType> should match the dtype passed to ArrayValue<Dtype>. In other words, I'd like to avoid the risk of the two becoming out of sync — e.g. Dataset<ArrayShape, NumericLikeType> and ArrayValue<NumericType>. This is why I refer to the type of the dataset prop with Value<Prop['dataset'].

Perhaps the second goal is not critical, since TypeScript would show an error in the container if the dtype of dataset and value/auxValues did not match. So should I stick with ArrayValue<NumericLikeType>?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Opening a PR to propose a normalisation of the mapped components' props.

auxErrors?: (NumArray | undefined)[];
dims: number[];
dimMapping: DimensionMapping;
Expand Down Expand Up @@ -76,12 +77,13 @@ function MappedLineVis(props: Props) {
} = config;

const numArray = useToNumArray(value);
const numAuxArrays = useToNumArrays(auxValues);
const [slicedDims, slicedMapping] = useSlicedDimsAndMapping(dims, dimMapping);

const hookArgs = [slicedDims, slicedMapping] as const;
const dataArray = useMappedArray(numArray, ...hookArgs);
const errorArray = useMappedArray(errors, ...hookArgs);
const auxArrays = useMappedArrays(auxValues, ...hookArgs);
const auxArrays = useMappedArrays(numAuxArrays, ...hookArgs);
const auxErrorsArrays = useMappedArrays(auxErrors, ...hookArgs);

const dataDomain = useDomain(
Expand Down Expand Up @@ -114,7 +116,6 @@ function MappedLineVis(props: Props) {
config={config}
getExportURL={
getExportURL &&
dataset &&
((format) => getExportURL(format, dataset, selection, value))
}
/>,
Expand All @@ -135,7 +136,7 @@ function MappedLineVis(props: Props) {
}}
ordinateLabel={valueLabel}
title={title}
dtype={dataset && formatNumLikeType(dataset.type)}
dtype={formatNumLikeType(dataset.type)}
errorsArray={errorArray}
showErrors={showErrors}
auxiliaries={auxArrays.map((array, i) => ({
Expand Down
11 changes: 8 additions & 3 deletions packages/app/src/vis-packs/nexus/NxSignalPicker.tsx
Original file line number Diff line number Diff line change
@@ -1,14 +1,19 @@
import { type ComplexType, type NumericType } from '@h5web/shared/hdf5-models';
import {
type ComplexType,
type NumericLikeType,
} from '@h5web/shared/hdf5-models';

import { type DatasetDef } from './models';
import styles from './NxSignalPicker.module.css';

interface Props<T extends NumericType | ComplexType> {
interface Props<T extends NumericLikeType | ComplexType> {
definitions: DatasetDef<T>[];
onChange: (def: DatasetDef<T>) => void;
}

function NxSignalPicker<T extends NumericType | ComplexType>(props: Props<T>) {
function NxSignalPicker<T extends NumericLikeType | ComplexType>(
props: Props<T>,
) {
const { definitions, onChange } = props;

return (
Expand Down
11 changes: 8 additions & 3 deletions packages/app/src/vis-packs/nexus/NxValuesFetcher.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
import { type ComplexType, type NumericType } from '@h5web/shared/hdf5-models';
import {
type ComplexType,
type NumericLikeType,
} from '@h5web/shared/hdf5-models';
import { type ReactNode } from 'react';

import {
Expand All @@ -8,13 +11,15 @@ import {
} from '../core/hooks';
import { type NxData, type NxValues } from './models';

interface Props<T extends NumericType | ComplexType> {
interface Props<T extends NumericLikeType | ComplexType> {
nxData: NxData<T>;
selection?: string; // for slice-by-slice fetching
render: (val: NxValues<T>) => ReactNode;
}

function NxValuesFetcher<T extends NumericType | ComplexType>(props: Props<T>) {
function NxValuesFetcher<T extends NumericLikeType | ComplexType>(
props: Props<T>,
) {
const { nxData, selection, render } = props;
const { signalDef, axisDefs, auxDefs, titleDataset } = nxData;

Expand Down
45 changes: 40 additions & 5 deletions packages/app/src/vis-packs/nexus/guards.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,37 +2,72 @@ import {
assertComplexType,
assertNumericLikeType,
assertNumericType,
isDefined,
} from '@h5web/shared/guards';
import { type ComplexType, type NumericType } from '@h5web/shared/hdf5-models';
import {
type ComplexType,
type NumericLikeType,
type NumericType,
} from '@h5web/shared/hdf5-models';

import { type NxData } from './models';

export function assertNumericLikeNxData(
nxData: NxData,
): asserts nxData is NxData<NumericType> {
const { signalDef, auxDefs } = nxData;
): asserts nxData is NxData<NumericLikeType> {
const { signalDef, auxDefs, axisDefs } = nxData;

assertNumericLikeType(signalDef.dataset);

auxDefs.forEach((def) => {
assertNumericLikeType(def.dataset);
});

if (signalDef.errorDataset) {
assertNumericType(signalDef.errorDataset);
}

axisDefs.filter(isDefined).forEach((def) => {
assertNumericType(def.dataset);
});
loichuder marked this conversation as resolved.
Show resolved Hide resolved
}

export function assertNumericNxData(
nxData: NxData,
): asserts nxData is NxData<NumericType> {
const { signalDef, auxDefs } = nxData;
const { signalDef, auxDefs, axisDefs } = nxData;

assertNumericType(signalDef.dataset);

auxDefs.forEach((def) => {
assertNumericType(def.dataset);
});

if (signalDef.errorDataset) {
assertNumericType(signalDef.errorDataset);
}

axisDefs.filter(isDefined).forEach((def) => {
assertNumericType(def.dataset);
});
}

export function assertComplexNxData(
nxData: NxData,
): asserts nxData is NxData<ComplexType> {
const { signalDef, auxDefs } = nxData;
const { signalDef, auxDefs, axisDefs } = nxData;

assertComplexType(signalDef.dataset);

auxDefs.forEach((def) => {
assertComplexType(def.dataset);
});

if (signalDef.errorDataset) {
assertNumericType(signalDef.errorDataset);
}

axisDefs.filter(isDefined).forEach((def) => {
assertNumericType(def.dataset);
});
}
4 changes: 2 additions & 2 deletions packages/app/src/vis-packs/nexus/hooks.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import {
type ComplexType,
type GroupWithChildren,
type NumericType,
type NumericLikeType,
} from '@h5web/shared/hdf5-models';

import { type DimensionMapping } from '../../dimension-mapper/models';
Expand Down Expand Up @@ -48,7 +48,7 @@ export function useNxData(group: GroupWithChildren): NxData {
};
}

export function useNxImageDataToFetch<T extends NumericType | ComplexType>(
export function useNxImageDataToFetch<T extends NumericLikeType | ComplexType>(
nxData: NxData<T>,
selectedDef: DatasetDef<T>,
): NxData<T> {
Expand Down
2 changes: 1 addition & 1 deletion packages/app/src/vis-packs/nexus/models.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ export interface NxData<
silxStyle: SilxStyle;
}

export interface NxValues<T extends NumericType | ComplexType> {
export interface NxValues<T extends NumericLikeType | ComplexType> {
title: string;
signal: ArrayValue<T>;
errors?: NumArray;
Expand Down
18 changes: 18 additions & 0 deletions packages/shared/src/mock-values.ts
Original file line number Diff line number Diff line change
Expand Up @@ -266,6 +266,24 @@ export const mockValues = {
].flat(1),
[2, 2],
),
secondary_bool: () => {
const { data: dataOneDBool } = oneD_bool();
return ndarray(
dataOneDBool.flatMap((rowBool) =>
dataOneDBool.map((colBool) => (rowBool ? !colBool : colBool)),
),
[10, 10],
);
},
secondary_enum: () => {
const { data: dataOneDEnum } = oneD_enum();
return ndarray(
dataOneDEnum.flatMap((rowEnum) =>
dataOneDEnum.map((colEnum, j) => (j % 2 === 0 ? colEnum : rowEnum)),
),
[10, 10],
);
},
tertiary: () =>
ndarray(
twoD().data.map((v) => v / 2),
Expand Down
Loading