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

Enforce stricter types for formatters in MappedMatrixVis #1703

Closed
wants to merge 1 commit into from

Conversation

axelboc
Copy link
Contributor

@axelboc axelboc commented Sep 3, 2024

Follows #1702

As planned, the problem became much simpler: all the typing complexity related to formatters in MappedMatrixVis can now be moved into the getFormatter utility function, now called getCellFormatter.

I add a type guard called assertNdArrayValue that allows narrowing down an NdArray given an already narrowed-down DType. After this type guard, dataArray.get(row, col) is properly typed and we can pass the value to a strictly typed formatter without any casting.

I'll do something similar in ScalarVis.


The broader issue is that every time we narrow down a dataset object (e.g. Dataset<ArrayShape, PrintableType> => Dataset<ArrayShape, EnumType>), we have to narrow down the value right after with assertArrayValue/assertNdArrayValue/assertPrimitiveValue (e.g. ArrayValue<PrintableType> => ArrayValue<EnumType>). This is because we don't store dataset values within dataset objects. Maybe I'll try to tackle this one day.

@axelboc
Copy link
Contributor Author

axelboc commented Sep 4, 2024

Was a bit too quick on this one: it doesn't work at all with printable compound datasets...

Ideally, compound ndarrays would be typed with a tuple (NdArray<[Primitive<Field0Type>, Primitive<Field1Type>, ...]>) so that the return type of compoundNdArray.get(0) would be automatically inferred as Primitive<Field0Type> (which we could pass to a strictly typed formatter: ValueFormatter<Field0Type>).

Unfortunately, this doesn't work: the return type of compoundNdArray.get(0) is always inferred as Primitive<Field0Type> | Primitive<Field1Type> | .... It's a limitation of the ndarray library and I'm not sure it's fixable.

Seems that we're stuck with returning a weakly typed formatter from getFormatter (ValueFormatter<PrintableType>). Anyways, at least the typing weakness no longer leaks into MatrixVis, so it wasn't all worthless.

@axelboc axelboc closed this Sep 4, 2024
@axelboc axelboc deleted the strict-mapped-matrix branch September 4, 2024 10:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant