Skip to content
New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Remove obsolete type guards and fix axes attribute creation #379

Merged
merged 2 commits into from
Dec 11, 2020
Merged

Remove obsolete type guards and fix axes attribute creation #379

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1d0bc89
Remove obsolete type guards
axelboc Dec 11, 2020
9fe12db
Fix creation of `axes` attribute in mock data
axelboc Dec 11, 2020
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
4 changes: 2 additions & 2 deletions src/h5web/metadata-viewer/EntityTable.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import React, { ReactElement } from 'react';
import type { MyHDF5Entity } from '../providers/models';
import styles from './MetadataViewer.module.css';
import {
hasMySimpleShape,
hasSimpleShape,
isDataset,
isDatatype,
isLink,
Expand Down Expand Up @@ -64,7 +64,7 @@ function EntityTable(props: Props): ReactElement {
<tr>
<th scope="row">Shape</th>
<td>
{hasMySimpleShape(entity)
{hasSimpleShape(entity)
? renderShapeDims(entity.shape.dims)
: entity.shape.class}
</td>
Expand Down
8 changes: 2 additions & 6 deletions src/h5web/providers/mock/utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,5 @@
import { mockValues, mockMetadata } from './data';
import {
assertDataset,
assertMySimpleShape,
assertNumericType,
} from '../utils';
import { assertDataset, assertSimpleShape, assertNumericType } from '../utils';
import { assertArray, assertDefined } from '../../visualizations/shared/utils';
import ndarray from 'ndarray';
import { getEntityAtPath } from '../../explorer/utils';
Expand All @@ -13,7 +9,7 @@ export function getMockDataArray(path: string): ndarray {
assertDefined(dataset, `Expected entity at path "${path}"`);
assertDataset(dataset, `Expected group at path "${path}"`);
assertNumericType(dataset);
assertMySimpleShape(dataset);
assertSimpleShape(dataset);

const value = mockValues[dataset.id as keyof typeof mockValues];
assertArray<number>(value);
Expand Down
10 changes: 6 additions & 4 deletions src/h5web/providers/my-utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,12 @@ import {
MyHDF5ResolvedEntity,
} from '../providers/models';
import { NxInterpretation, SilxStyle } from '../visualizations/nexus/models';
import { makeSimpleShape, makeStrAttr } from './raw-utils';
import { makeNxAxesAttr, makeSimpleShape, makeStrAttr } from './raw-utils';

type EntityOpts = Partial<
Pick<MyHDF5ResolvedEntity, 'id' | 'attributes' | 'rawLink'>
>;
type GroupOpts = EntityOpts & { children?: MyHDF5Entity[] };
type AllOrNone<T> = T | { [K in keyof T]?: never };

export function makeMyGroup(
name: string,
Expand Down Expand Up @@ -149,7 +148,10 @@ export function makeMyNxDataGroup<
errors?: MyHDF5Dataset<HDF5SimpleShape, HDF5NumericType>;
title?: MyHDF5Dataset<HDF5ScalarShape, HDF5StringType>;
silxStyle?: SilxStyle;
} & AllOrNone<{ axes: T; axesAttr: (keyof T | '.')[] }> &
} & (
| { axes: T; axesAttr: (Extract<keyof T, string> | '.')[] }
| { axes?: never; axesAttr?: never }
) &
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changing my mind on this. I think inlining is clearer after all. 🤷‍♂️

The change below also required a slight change in typing: Extract<keyof T, string> instead of keyof T, as keyof T resolves to string | number | Symbol as explained in microsoft/TypeScript#23724 (comment) ... even though T is Record<string, ...>! This seems very odd... there's a comment about it in the linked PR, but no answer.

Copy link
Member

Choose a reason for hiding this comment

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

Probably keyof does not behave well with templates ? 🤷

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Generics you mean? The guy's comment on microsoft/TypeScript#23724 (comment) doesn't mention a generic so I don't think it's related.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Link to the comment: microsoft/TypeScript#23724 (comment)

GroupOpts
): MyHDF5Group {
const {
Expand All @@ -176,7 +178,7 @@ export function makeMyNxDataGroup<
...(groupOpts.attributes || []),
makeStrAttr('NX_class', 'NXdata'),
makeStrAttr('signal', signal.name),
makeStrAttr('axes', axesAttr),
...(axesAttr ? [makeNxAxesAttr(axesAttr)] : []),
axelboc marked this conversation as resolved.
Show resolved Hide resolved
...(silxStyle
? [makeStrAttr('SILX_style', JSON.stringify(silxStyle))]
: []),
Expand Down
2 changes: 1 addition & 1 deletion src/h5web/providers/raw-utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ export function makeAttr(
return { name, type, shape, value };
}

export function makeStrAttr(name: string, value: HDF5Value): HDF5Attribute {
export function makeStrAttr(name: string, value: string): HDF5Attribute {
return makeAttr(name, scalarShape, stringType, value);
}

Expand Down
23 changes: 2 additions & 21 deletions src/h5web/providers/utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import {
HDF5Type,
HDF5TypeClass,
HDF5RootLink,
HDF5Dataset,
HDF5ScalarShape,
HDF5NumericType,
MyHDF5Entity,
Expand Down Expand Up @@ -58,13 +57,7 @@ export function isLink(entity: MyHDF5Entity): entity is MyHDF5Link {
return entity.kind === MyHDF5EntityKind.Link;
}

function hasSimpleShape<T extends HDF5Type>(
dataset: HDF5Dataset<HDF5Shape, T>
): dataset is HDF5Dataset<HDF5SimpleShape, T> {
return dataset.shape.class === HDF5ShapeClass.Simple;
}

export function hasMySimpleShape<T extends HDF5Type>(
export function hasSimpleShape<T extends HDF5Type>(
dataset: MyHDF5Dataset<HDF5Shape, T>
): dataset is MyHDF5Dataset<HDF5SimpleShape, T> {
return dataset.shape.class === HDF5ShapeClass.Simple;
Expand Down Expand Up @@ -123,21 +116,9 @@ export function assertNumericType<S extends HDF5Shape>(
}

export function assertSimpleShape<T extends HDF5Type>(
dataset: HDF5Dataset<HDF5Shape, T>
): asserts dataset is HDF5Dataset<HDF5SimpleShape, T> {
if (!hasSimpleShape(dataset)) {
throw new Error('Expected dataset to have simple shape');
}

if (dataset.shape.dims.length === 0) {
throw new Error('Expected dataset with simple shape to have dimensions');
}
}
loichuder marked this conversation as resolved.
Show resolved Hide resolved

export function assertMySimpleShape<T extends HDF5Type>(
dataset: MyHDF5Dataset<HDF5Shape, T>
): asserts dataset is MyHDF5Dataset<HDF5SimpleShape, T> {
if (!hasMySimpleShape(dataset)) {
if (!hasSimpleShape(dataset)) {
throw new Error('Expected dataset to have simple shape');
}

Expand Down
4 changes: 2 additions & 2 deletions src/h5web/visualizations/containers/HeatmapVisContainer.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,15 @@ import { useDatasetValue } from './hooks';
import {
assertDataset,
assertNumericType,
assertMySimpleShape,
assertSimpleShape,
} from '../../providers/utils';
import MappedHeatmapVis from '../heatmap/MappedHeatmapVis';
import type { VisContainerProps } from './models';

function HeatmapVisContainer(props: VisContainerProps): ReactElement {
const { entity } = props;
assertDataset(entity);
assertMySimpleShape(entity);
assertSimpleShape(entity);
assertNumericType(entity);

const { dims } = entity.shape;
Expand Down
4 changes: 2 additions & 2 deletions src/h5web/visualizations/containers/LineVisContainer.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,15 @@ import { useDatasetValue } from './hooks';
import {
assertDataset,
assertNumericType,
assertMySimpleShape,
assertSimpleShape,
} from '../../providers/utils';
import MappedLineVis from '../line/MappedLineVis';
import type { VisContainerProps } from './models';

function LineVisContainer(props: VisContainerProps): ReactElement {
const { entity } = props;
assertDataset(entity);
assertMySimpleShape(entity);
assertSimpleShape(entity);
assertNumericType(entity);

const value = useDatasetValue(entity.id);
Expand Down
4 changes: 2 additions & 2 deletions src/h5web/visualizations/containers/MatrixVisContainer.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
import React, { ReactElement } from 'react';
import { useDatasetValue } from './hooks';
import { assertDataset, assertMySimpleShape } from '../../providers/utils';
import { assertDataset, assertSimpleShape } from '../../providers/utils';
import MappedMatrixVis from '../matrix/MappedMatrixVis';
import type { VisContainerProps } from './models';

function MatrixVisContainer(props: VisContainerProps): ReactElement {
const { entity } = props;
assertDataset(entity);
assertMySimpleShape(entity);
assertSimpleShape(entity);

const value = useDatasetValue(entity.id);
if (!value) {
Expand Down
8 changes: 4 additions & 4 deletions src/h5web/visualizations/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import type { MyHDF5Entity } from '../providers/models';
import {
hasScalarShape,
hasBaseType,
hasMySimpleShape,
hasSimpleShape,
hasNumericType,
isDataset,
isGroup,
Expand Down Expand Up @@ -72,7 +72,7 @@ export const VIS_DEFS: Record<Vis, VisDef> = {
return (
isDataset(entity) &&
hasBaseType(entity) &&
hasMySimpleShape(entity) &&
hasSimpleShape(entity) &&
entity.shape.dims.length >= 1
);
},
Expand All @@ -86,7 +86,7 @@ export const VIS_DEFS: Record<Vis, VisDef> = {
return (
isDataset(entity) &&
hasNumericType(entity) &&
hasMySimpleShape(entity) &&
hasSimpleShape(entity) &&
entity.shape.dims.length >= 1
);
},
Expand All @@ -100,7 +100,7 @@ export const VIS_DEFS: Record<Vis, VisDef> = {
return (
isDataset(entity) &&
hasNumericType(entity) &&
hasMySimpleShape(entity) &&
hasSimpleShape(entity) &&
entity.shape.dims.length >= 2
);
},
Expand Down
4 changes: 2 additions & 2 deletions src/h5web/visualizations/nexus/utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import {
assertGroup,
assertDataset,
assertNumericType,
assertMySimpleShape,
assertSimpleShape,
} from '../../providers/utils';
import {
NxAttribute,
Expand Down Expand Up @@ -85,7 +85,7 @@ export function findSignalDataset(
assertDefined(dataset, `Expected "${signalName}" signal entity to exist`);
assertDataset(dataset, `Expected "${signalName}" signal to be a dataset`);
assertNumericType(dataset);
assertMySimpleShape(dataset);
assertSimpleShape(dataset);

return dataset;
}
Expand Down