[Feature] Remove implicit Float default type in vtkDataArray in favor of an explicit Error

[sedghi]

Motivation

vtkDataArray implicitly assumes a default data type of Float, which I believe is a mistake.

const DEFAULT_VALUES = {
  dataType: DefaultDataType,
};

While debugging a deep issue in Cornerstone, I discovered my mistake in my code:

const scalarArray = vtkDataArray.newInstance({
  name: 'Pixels',
  numberOfComponents: 1,
  values: [...newPixelData],
});

The data type defaulted to float, which caused problems with rendering floats in labelmap rendering with colors and outlines. Once I defined a typed array correctly, the issue was resolved.

I don't think there are many implicit assumptions elsewhere in vtk. I strongly support explicitly throwing an error or defining a typed array instead of using an array of numbers.

What do you think?

Detailed Description

Above

[finetjul]

(Not only the type was not correct, but in your example you alos made one copy too many...)

Regarding your suggestion, I don't have a preference.

[sedghi]

Copying may or may not be an issue depending on the use case, but the implicit assumption of using Float is somewhat unusual compared to the rest of the vtk.js library. I don't believe we have other instances with default types, at least not in such an important context as vtkDataArray.

[jourdain]

VTK use float(32) by default for its points unless stated otherwise. As we only have 1 generic vtkDataArray we picked the same default. Are you saying we should remove the default and through an error if a typed array is not provided?

[sedghi]

Can vtkDataArray represent something other than points, like scalar data for imageData? If so, I think the default float32 might be problematic, causing bugs that could be easily caught by an error if not typed array

[jourdain]

We will break a lot of apps out there that are fine to that float conversion. But that could be rolled into a breaking change release.

@floryst what do you think?

[floryst]

How about logging a console warning about defaulting to Float32Array if Array.isArray(values) and a dataType is not explicitly provided? The docs could also be improved to communicate the defaults when users provide a plain JS array. I agree it would be better to only accept typed arrays so the user can be explicit about the desired data type, but I want to look at alternatives before making this a breaking change.

[sedghi]

Warning is good for now, but maybe in future If you have other changes that might cause many apps to crash, consider bundling this one with them in the same version.