Simplify MatrixVis API

[axelboc]

I've been looking into making the types of getFormatter in MappedMatrixVis more strict. I'd like to ensure that, for instance, when we have a numeric array:

• the formatter is typed as (val: number) => string (instead of (val: number | string | ...) => string);
• we can't pass a string formatter with a numeric array.

Unfortunately, I keep hitting roadblocks. While investigating, I found an interesting refactoring of MatrixVis that simplifies a bunch of things and may remove at least one of those roadblocks...

The gist is that I remove the dataArray prop and let the consumer retrieve the value using the row and column index. This means we no longer have to worry about the formatter val parameter matching the type of the values in the array, whether it be when using MatrixVis or typing its props. We still have to worry about it in MappedMatrixVis, though, but hopefully I'll find a way to fix that next.

Breaking changes: replace props dataArray and formatter with props dims and cellFormatter. Typical refactoring:

const arr = ndarray([0, 1, 2, 3]);

// BEFORE
<MatrixVis
  dataArray={arr}
  formatter={(val /* number | string | ... */) => (val as number).toFixed(2)}
  /* ... */
/>

// AFTER
<MatrixVis
  dims={arr.shape}
  cellFormatter={(row, col) => arr.get(row, col).toFixed(2)}
  /* ... */
/>
    ```

[axelboc]

This logic was not needed since dataArray.get(row, 0) works fine with 1D arrays.

[axelboc]

Unrelated refactoring here: I'm just opening the door to asserting compound types more strictly in the future.

[axelboc]

Adding this assertion to at least check that compound values are arrays.

[axelboc]

Swapping the logic because type was narrowed down to never in the else branch due to the fact that ScalarShape ⊂ ArrayShape ([] ⊂ number[]). I have a note somewhere to try to change ArrayShape to [number, ...number[]], though I have to check if it's worth the effort.

[loichuder]

Not too sure.

I find that it should not be the responsability of the MatrixVis consumer to deal with the data access to the dataArray. Notably, it would be easy for an unwary user to invert row and col.

About the formatter type, I guess you tried something like

interface Props<T extends PrintableType> {
  dataArray: NdArray<ArrayValue<T>>
  formatter: (value: Primitive<T>, colIndex: number) => string;
...
}

and it didn't work ?

[axelboc]

Yeah, doesn't work inside the MatrixVis component itself. And that's without even trying to distribute the PrintableType union to get formatter: (val: number) => string | (val: string) => string | ... instead of formatter: (val: number | string | ...) => string...

I don't know, I feel like trying to hide away dataArray.get() is unnecessary. It also forces consumers to create the dataArray (or a view of it) in a certain way (cols, rows) ... but this restriction is artificial. With the new cellFormatter, the question for the consumer simply becomes: "what text do I want to display in cell (col, row)". You don't even need an ndarray to answer this...

I know removing the dataArray prop breaks the consistency between our "vis" components, but was this consistency really meaningful in the first place?

[loichuder]

I don't know, I feel like trying to hide away dataArray.get() is unnecessary. It also forces consumers to create the dataArray (or a view of it) in a certain way (cols, rows) ... but this restriction is artificial. With the new cellFormatter, the question for the consumer simply becomes: "what text do I want to display in cell (col, row)". You don't even need an ndarray to answer this..

Yeah good point. Let's try it.