From afe2468d8a525f1fa7ae0f2eff621afba694be19 Mon Sep 17 00:00:00 2001 From: Axel Bocciarelli Date: Wed, 3 Nov 2021 14:53:10 +0100 Subject: [PATCH 1/7] Minor typing improvements --- packages/app/src/vis-packs/core/hooks.ts | 8 +++----- packages/app/src/vis-packs/core/utils.ts | 12 ++++++------ .../nexus/containers/NxComplexImageContainer.tsx | 14 +++++--------- .../nexus/containers/NxImageContainer.tsx | 13 +++++-------- .../vis-packs/nexus/containers/NxRgbContainer.tsx | 8 ++++---- packages/app/src/vis-packs/nexus/utils.ts | 13 +++++++++++++ 6 files changed, 36 insertions(+), 32 deletions(-) diff --git a/packages/app/src/vis-packs/core/hooks.ts b/packages/app/src/vis-packs/core/hooks.ts index 2d876d988..44d9121a7 100644 --- a/packages/app/src/vis-packs/core/hooks.ts +++ b/packages/app/src/vis-packs/core/hooks.ts @@ -95,14 +95,12 @@ export const useCombinedDomain = createMemo(getCombinedDomain); const useBaseArray = createMemo(getBaseArray); const useApplyMapping = createMemo(applyMapping); -export function useMappedArray( - value: T, +export function useMappedArray( + value: U, dims: number[], mapping: DimensionMapping, autoScale?: boolean -): T extends (infer U)[] - ? [NdArray, NdArray] - : [undefined, undefined]; +): U extends T[] ? [NdArray, NdArray] : [undefined, undefined]; export function useMappedArray( value: T[] | undefined, diff --git a/packages/app/src/vis-packs/core/utils.ts b/packages/app/src/vis-packs/core/utils.ts index 1b01d0f5c..042c10c89 100644 --- a/packages/app/src/vis-packs/core/utils.ts +++ b/packages/app/src/vis-packs/core/utils.ts @@ -10,10 +10,10 @@ import { getAttributeValue } from '../../utils'; export const DEFAULT_DOMAIN: Domain = [0.1, 1]; -export function getBaseArray( - value: T, +export function getBaseArray( + value: U, rawDims: number[] -): T extends (infer U)[] ? NdArray : undefined; +): U extends T[] ? NdArray : undefined; export function getBaseArray( value: T[] | undefined, @@ -22,10 +22,10 @@ export function getBaseArray( return value && ndarray(value, rawDims); } -export function applyMapping | undefined>( - baseArray: T, +export function applyMapping | undefined>( + baseArray: U, mapping: (number | Axis | ':')[] -): T extends NdArray ? NdArray : undefined; +): U extends NdArray ? U : undefined; export function applyMapping( baseArray: NdArray | undefined, diff --git a/packages/app/src/vis-packs/nexus/containers/NxComplexImageContainer.tsx b/packages/app/src/vis-packs/nexus/containers/NxComplexImageContainer.tsx index cf2c80758..52858d278 100644 --- a/packages/app/src/vis-packs/nexus/containers/NxComplexImageContainer.tsx +++ b/packages/app/src/vis-packs/nexus/containers/NxComplexImageContainer.tsx @@ -1,9 +1,4 @@ -import type { H5WebComplex } from '@h5web/shared'; -import { - assertComplexType, - assertGroupWithChildren, - assertMinDims, -} from '@h5web/shared'; +import { assertGroupWithChildren, assertMinDims } from '@h5web/shared'; import DimensionMapper from '../../../dimension-mapper/DimensionMapper'; import VisBoundary from '../../VisBoundary'; @@ -11,15 +6,16 @@ import MappedComplexVis from '../../core/complex/MappedComplexVis'; import { useDimMappingState } from '../../hooks'; import type { VisContainerProps } from '../../models'; import NxValuesFetcher from '../NxValuesFetcher'; -import { getNxData, getDatasetLabel } from '../utils'; +import { getNxData, getDatasetLabel, assertComplexSignal } from '../utils'; function NxComplexImageContainer(props: VisContainerProps) { const { entity } = props; assertGroupWithChildren(entity); const nxData = getNxData(entity); + assertComplexSignal(nxData); + const { signalDataset, silxStyle } = nxData; - assertComplexType(signalDataset); assertMinDims(signalDataset, 2); const { shape: dims } = signalDataset; @@ -40,7 +36,7 @@ function NxComplexImageContainer(props: VisContainerProps) { const { signal, axisMapping, title } = nxValues; return ( { + assertNumericType(nxData.signalDataset); +} + +export function assertComplexSignal( + nxData: NxData +): asserts nxData is NxData { + assertComplexType(nxData.signalDataset); +} From 8c24b46d113721554886f5eb5b7a66af64d0a15b Mon Sep 17 00:00:00 2001 From: Axel Bocciarelli Date: Wed, 3 Nov 2021 10:54:40 +0100 Subject: [PATCH 2/7] Assert provided value matches dataset type/entity --- packages/app/src/vis-packs/core/hooks.ts | 15 +++++- packages/shared/src/guards.ts | 61 ++++++++++++++++++++++++ 2 files changed, 74 insertions(+), 2 deletions(-) diff --git a/packages/app/src/vis-packs/core/hooks.ts b/packages/app/src/vis-packs/core/hooks.ts index 44d9121a7..a257c95e6 100644 --- a/packages/app/src/vis-packs/core/hooks.ts +++ b/packages/app/src/vis-packs/core/hooks.ts @@ -5,6 +5,7 @@ import { ScaleType, getBounds, getValidDomainForScale, + assertDatasetValue, } from '@h5web/shared'; import type { NdArray } from 'ndarray'; import { useContext, useMemo } from 'react'; @@ -46,10 +47,13 @@ export function useDatasetValue( } // If `dimMapping` is not provided or has no slicing dimension, the entire dataset will be fetched - return valuesStore.get({ + const value = valuesStore.get({ path: dataset.path, selection: getSliceSelection(dimMapping), }); + + assertDatasetValue(value, dataset); + return value; } export function useDatasetValues( @@ -60,7 +64,14 @@ export function useDatasetValues(datasets: Dataset[]): Record { const { valuesStore } = useContext(ProviderContext); return Object.fromEntries( - datasets.map(({ name, path }) => [name, valuesStore.get({ path })]) + datasets.map((dataset) => { + const { name, path } = dataset; + + const value = valuesStore.get({ path }); + assertDatasetValue(value, dataset); + + return [name, value]; + }) ); } diff --git a/packages/shared/src/guards.ts b/packages/shared/src/guards.ts index 97a1b9542..73a9173c3 100644 --- a/packages/shared/src/guards.ts +++ b/packages/shared/src/guards.ts @@ -19,6 +19,8 @@ import type { ComplexType, PrintableType, StringType, + Primitive, + Value, } from './models-hdf5'; import { ScaleType } from './models-vis'; import { toArray } from './utils'; @@ -59,6 +61,18 @@ export function assertDefined( } } +function assertNum(val: unknown): asserts val is number { + if (typeof val !== 'number') { + throw new TypeError('Expected number'); + } +} + +function assertBool(val: unknown): asserts val is boolean { + if (typeof val !== 'boolean') { + throw new TypeError('Expected boolean'); + } +} + export function assertStr( val: unknown, message = 'Expected string' @@ -68,6 +82,17 @@ export function assertStr( } } +function assertComplex(val: unknown): asserts val is H5WebComplex { + if ( + !Array.isArray(val) || + val.length !== 2 || + typeof val[0] !== 'number' || + typeof val[1] !== 'number' + ) { + throw new TypeError('Expected complex'); + } +} + export function assertArray(val: unknown): asserts val is T[] { if (!Array.isArray(val)) { throw new TypeError('Expected array'); @@ -219,6 +244,10 @@ export function assertNumericType( } } +export function isStringType(type: DType): type is StringType { + return type.class === DTypeClass.String; +} + export function hasComplexType( dataset: Dataset ): dataset is Dataset { @@ -267,6 +296,38 @@ export function isComplexValue( return type.class === DTypeClass.Complex; } +function assertPrimitiveValue>( + dataset: D, + value: unknown +): asserts value is Primitive { + if (hasNumericType(dataset)) { + assertNum(value); + } else if (hasStringType(dataset)) { + assertStr(value); + } else if (hasBoolType(dataset)) { + assertBool(value); + } else if (hasComplexType(dataset)) { + assertComplex(value); + } +} + +export function assertDatasetValue( + value: unknown, + dataset: D +): asserts value is Value { + // Array shape + if (hasArrayShape(dataset)) { + assertArray(value); + + if (value.length > 0) { + assertPrimitiveValue(dataset, value[0]); + } + } else if (!hasNonNullShape(dataset)) { + // Scalar shape + assertPrimitiveValue(dataset, value); + } +} + export function assertDataLength( arr: NdArray | number[] | undefined, dataArray: NdArray | number[], From f3340bd489e43a9f0b2e2d41b9c081f621f0a4fc Mon Sep 17 00:00:00 2001 From: Axel Bocciarelli Date: Thu, 28 Oct 2021 12:06:03 +0200 Subject: [PATCH 3/7] Fix scalar values provided by H5Grove --- packages/app/src/providers/h5grove/h5grove-api.ts | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/packages/app/src/providers/h5grove/h5grove-api.ts b/packages/app/src/providers/h5grove/h5grove-api.ts index 02bef1056..8e7ad69aa 100644 --- a/packages/app/src/providers/h5grove/h5grove-api.ts +++ b/packages/app/src/providers/h5grove/h5grove-api.ts @@ -6,7 +6,12 @@ import type { GroupWithChildren, UnresolvedEntity, } from '@h5web/shared'; -import { assertDataset, buildEntityPath, EntityKind } from '@h5web/shared'; +import { + hasScalarShape, + assertDataset, + buildEntityPath, + EntityKind, +} from '@h5web/shared'; import { isString } from 'lodash'; import { ProviderApi } from '../api'; @@ -51,7 +56,8 @@ export class H5GroveApi extends ProviderApi { } const buffer = await this.fetchBinaryData(params); - return new DTypedArray(buffer); + const array = new DTypedArray(buffer); + return hasScalarShape(entity) ? array[0] : array; } private async fetchEntity(path: string): Promise { From b641125ae403324c6a9629361f325af4badc8d4e Mon Sep 17 00:00:00 2001 From: Axel Bocciarelli Date: Tue, 2 Nov 2021 16:56:53 +0100 Subject: [PATCH 4/7] Fix non-numeric arrays not flattened in H5Grove provider --- .../app/src/providers/h5grove/h5grove-api.ts | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/packages/app/src/providers/h5grove/h5grove-api.ts b/packages/app/src/providers/h5grove/h5grove-api.ts index 8e7ad69aa..042c14675 100644 --- a/packages/app/src/providers/h5grove/h5grove-api.ts +++ b/packages/app/src/providers/h5grove/h5grove-api.ts @@ -8,6 +8,7 @@ import type { } from '@h5web/shared'; import { hasScalarShape, + hasArrayShape, assertDataset, buildEntityPath, EntityKind, @@ -51,13 +52,16 @@ export class H5GroveApi extends ProviderApi { assertDataset(entity); const DTypedArray = typedArrayFromDType(entity.type); - if (!DTypedArray) { - return this.fetchData(params); + if (DTypedArray) { + const buffer = await this.fetchBinaryData(params); + const array = new DTypedArray(buffer); + return hasScalarShape(entity) ? array[0] : array; } - const buffer = await this.fetchBinaryData(params); - const array = new DTypedArray(buffer); - return hasScalarShape(entity) ? array[0] : array; + const value = await this.fetchData(params); + return hasArrayShape(entity) + ? (value as unknown[]).flat(entity.shape.length - 1) + : value; } private async fetchEntity(path: string): Promise { @@ -120,7 +124,7 @@ export class H5GroveApi extends ProviderApi { params: ValuesStoreParams ): Promise { const { data } = await this.cancellableFetchValue( - `/data/`, + '/data/', params, { ...params, format: 'bin' }, 'arraybuffer' From 8cb609738ec9fb559a0d46f60181e24643df34ea Mon Sep 17 00:00:00 2001 From: Axel Bocciarelli Date: Wed, 3 Nov 2021 11:09:17 +0100 Subject: [PATCH 5/7] Convert typed arrays back to arrays in H5Grove provider --- packages/app/src/providers/h5grove/h5grove-api.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/app/src/providers/h5grove/h5grove-api.ts b/packages/app/src/providers/h5grove/h5grove-api.ts index 042c14675..a66f01987 100644 --- a/packages/app/src/providers/h5grove/h5grove-api.ts +++ b/packages/app/src/providers/h5grove/h5grove-api.ts @@ -55,7 +55,7 @@ export class H5GroveApi extends ProviderApi { if (DTypedArray) { const buffer = await this.fetchBinaryData(params); const array = new DTypedArray(buffer); - return hasScalarShape(entity) ? array[0] : array; + return hasScalarShape(entity) ? array[0] : [...array]; } const value = await this.fetchData(params); From fa9708f288d488b408a40b3bd27cbb0982ffc089 Mon Sep 17 00:00:00 2001 From: Axel Bocciarelli Date: Wed, 3 Nov 2021 13:25:12 +0100 Subject: [PATCH 6/7] Ensure value of null-shape dataset is never fetched --- .../app/src/providers/h5grove/h5grove-api.ts | 2 ++ packages/app/src/providers/hsds/hsds-api.ts | 4 ++-- packages/app/src/providers/hsds/utils.ts | 18 +++++++++++++----- packages/app/src/vis-packs/core/hooks.ts | 7 ++++++- packages/shared/src/guards.ts | 5 ++--- 5 files changed, 25 insertions(+), 11 deletions(-) diff --git a/packages/app/src/providers/h5grove/h5grove-api.ts b/packages/app/src/providers/h5grove/h5grove-api.ts index a66f01987..88b2722f4 100644 --- a/packages/app/src/providers/h5grove/h5grove-api.ts +++ b/packages/app/src/providers/h5grove/h5grove-api.ts @@ -12,6 +12,7 @@ import { assertDataset, buildEntityPath, EntityKind, + assertNonNullShape, } from '@h5web/shared'; import { isString } from 'lodash'; @@ -50,6 +51,7 @@ export class H5GroveApi extends ProviderApi { const { path } = params; const entity = await this.getEntity(path); assertDataset(entity); + assertNonNullShape(entity); const DTypedArray = typedArrayFromDType(entity.type); if (DTypedArray) { diff --git a/packages/app/src/providers/hsds/hsds-api.ts b/packages/app/src/providers/hsds/hsds-api.ts index 5029241e9..da07e8460 100644 --- a/packages/app/src/providers/hsds/hsds-api.ts +++ b/packages/app/src/providers/hsds/hsds-api.ts @@ -33,7 +33,7 @@ import type { HsdsId, } from './models'; import { - assertHsdsDataset, + assertNonNullHsdsDataset, isHsdsGroup, convertHsdsShape, convertHsdsType, @@ -111,7 +111,7 @@ export class HsdsApi extends ProviderApi { const { path } = params; const entity = await this.getEntity(path); - assertHsdsDataset(entity); + assertNonNullHsdsDataset(entity); const value = await this.fetchValue(entity.id, params); diff --git a/packages/app/src/providers/hsds/utils.ts b/packages/app/src/providers/hsds/utils.ts index 372e66756..99dfa7a0c 100644 --- a/packages/app/src/providers/hsds/utils.ts +++ b/packages/app/src/providers/hsds/utils.ts @@ -8,8 +8,16 @@ import type { ComplexType, DType, Attribute, + ScalarShape, + ArrayShape, +} from '@h5web/shared'; +import { + isGroup, + isDataset, + DTypeClass, + Endianness, + hasNonNullShape, } from '@h5web/shared'; -import { isGroup, isDataset, DTypeClass, Endianness } from '@h5web/shared'; import type { HsdsType, @@ -34,11 +42,11 @@ export function assertHsdsEntity(entity: Entity): asserts entity is HsdsEntity { } } -export function assertHsdsDataset( +export function assertNonNullHsdsDataset( entity: HsdsEntity -): asserts entity is HsdsEntity { - if (!isHsdsDataset(entity)) { - throw new Error('Expected entity to be HSDS dataset'); +): asserts entity is HsdsEntity> { + if (!isHsdsDataset(entity) || !hasNonNullShape(entity)) { + throw new Error('Expected entity to be HSDS dataset with non-null shape'); } } diff --git a/packages/app/src/vis-packs/core/hooks.ts b/packages/app/src/vis-packs/core/hooks.ts index a257c95e6..f939f1856 100644 --- a/packages/app/src/vis-packs/core/hooks.ts +++ b/packages/app/src/vis-packs/core/hooks.ts @@ -6,6 +6,7 @@ import { getBounds, getValidDomainForScale, assertDatasetValue, + assertNonNullShape, } from '@h5web/shared'; import type { NdArray } from 'ndarray'; import { useContext, useMemo } from 'react'; @@ -46,6 +47,9 @@ export function useDatasetValue( return undefined; } + // Dataset with null shape has no value to fetch + assertNonNullShape(dataset); + // If `dimMapping` is not provided or has no slicing dimension, the entire dataset will be fetched const value = valuesStore.get({ path: dataset.path, @@ -65,8 +69,9 @@ export function useDatasetValues(datasets: Dataset[]): Record { return Object.fromEntries( datasets.map((dataset) => { - const { name, path } = dataset; + assertNonNullShape(dataset); + const { name, path } = dataset; const value = valuesStore.get({ path }); assertDatasetValue(value, dataset); diff --git a/packages/shared/src/guards.ts b/packages/shared/src/guards.ts index 73a9173c3..266caf134 100644 --- a/packages/shared/src/guards.ts +++ b/packages/shared/src/guards.ts @@ -311,18 +311,17 @@ function assertPrimitiveValue>( } } -export function assertDatasetValue( +export function assertDatasetValue>( value: unknown, dataset: D ): asserts value is Value { - // Array shape if (hasArrayShape(dataset)) { assertArray(value); if (value.length > 0) { assertPrimitiveValue(dataset, value[0]); } - } else if (!hasNonNullShape(dataset)) { + } else { // Scalar shape assertPrimitiveValue(dataset, value); } From 4399b7b79d46b028751c0a94c60a3cfd0011e575 Mon Sep 17 00:00:00 2001 From: Axel Bocciarelli Date: Wed, 3 Nov 2021 14:53:06 +0100 Subject: [PATCH 7/7] Fix flattening of values fetched with `selection` param --- packages/app/src/providers/h5grove/h5grove-api.ts | 6 +++--- packages/app/src/providers/hsds/hsds-api.ts | 8 ++------ packages/app/src/providers/utils.ts | 15 +++++++++++++-- 3 files changed, 18 insertions(+), 11 deletions(-) diff --git a/packages/app/src/providers/h5grove/h5grove-api.ts b/packages/app/src/providers/h5grove/h5grove-api.ts index 88b2722f4..998de22de 100644 --- a/packages/app/src/providers/h5grove/h5grove-api.ts +++ b/packages/app/src/providers/h5grove/h5grove-api.ts @@ -19,7 +19,7 @@ import { isString } from 'lodash'; import { ProviderApi } from '../api'; import type { ValuesStoreParams } from '../models'; import { ProviderError } from '../models'; -import { convertDtype, handleAxiosError } from '../utils'; +import { convertDtype, flattenValue, handleAxiosError } from '../utils'; import type { H5GroveAttribute, H5GroveAttrValuesResponse, @@ -48,7 +48,7 @@ export class H5GroveApi extends ProviderApi { public async getValue( params: ValuesStoreParams ): Promise { - const { path } = params; + const { path, selection } = params; const entity = await this.getEntity(path); assertDataset(entity); assertNonNullShape(entity); @@ -62,7 +62,7 @@ export class H5GroveApi extends ProviderApi { const value = await this.fetchData(params); return hasArrayShape(entity) - ? (value as unknown[]).flat(entity.shape.length - 1) + ? flattenValue(value, entity, selection) : value; } diff --git a/packages/app/src/providers/hsds/hsds-api.ts b/packages/app/src/providers/hsds/hsds-api.ts index da07e8460..42d85c1de 100644 --- a/packages/app/src/providers/hsds/hsds-api.ts +++ b/packages/app/src/providers/hsds/hsds-api.ts @@ -17,7 +17,7 @@ import { import { ProviderApi } from '../api'; import type { ValuesStoreParams } from '../models'; import { ProviderError } from '../models'; -import { handleAxiosError } from '../utils'; +import { flattenValue, handleAxiosError } from '../utils'; import type { HsdsDatasetResponse, HsdsDatatypeResponse, @@ -118,11 +118,7 @@ export class HsdsApi extends ProviderApi { // https://github.com/HDFGroup/hsds/issues/88 // HSDS does not reduce the number of dimensions when selecting indices // Therefore the flattening must be done on all dimensions regardless of the selection - if (hasArrayShape(entity)) { - return (value as unknown[]).flat(entity.shape.length - 1); - } - - return value; + return hasArrayShape(entity) ? flattenValue(value, entity) : value; } private async fetchRootId(): Promise { diff --git a/packages/app/src/providers/utils.ts b/packages/app/src/providers/utils.ts index 2248c4aba..a3cd0113a 100644 --- a/packages/app/src/providers/utils.ts +++ b/packages/app/src/providers/utils.ts @@ -1,5 +1,5 @@ -import type { DType } from '@h5web/shared'; -import { Endianness, DTypeClass } from '@h5web/shared'; +import type { ArrayShape, Dataset, DType } from '@h5web/shared'; +import { Endianness, DTypeClass, assertArray } from '@h5web/shared'; import axios from 'axios'; import type { ProviderError } from './models'; @@ -81,6 +81,17 @@ export function convertDtype(dtype: string): DType { } } +export function flattenValue( + value: unknown, + dataset: Dataset, + selection?: string +): unknown[] { + assertArray(value); + const slicedDims = selection?.split(',').filter((s) => s.includes(':')); + const dims = slicedDims || dataset.shape; + return value.flat(dims.length - 1); +} + export async function handleAxiosError( func: () => Promise, getErrorToThrow: (