Refactor datatype mapping #94
Conversation
|
|
||
| function obj = fromTensorstoreType(tensorstoreType) | ||
| % Create a datatype object based on Tensorstore datatype name | ||
| arguments |
There was a problem hiding this comment.
You don't need the arguments block and validatestrings. You could either remove the args block (which BTW means you will accept any inputs that are convertable to string, like double and datetime) or use the mustBeMember validator function. Nothing specifically wrong with this pattern, but it's a mix of two paradigms and feels weird. I'd prefer the more modern approach.
There was a problem hiding this comment.
That was my initial impulse as well! Unfortunately there seems to be a limitation:
MATLABType (1,1) string {mustBeMember(MATLABType, ZarrDatatype.MATLABTypes)} results in:
Line 59: For input arguments, validation functions must only use previously declared positional arguments, the argument being validated, or literals.
And it felt worse to me to have to re-list valid types again as a literal
There was a problem hiding this comment.
Ooof.
You can call mustBeMember directly. You could write a local function in the class that calls it with the right arguments.
function mustBeMATLABType(type)
mustBeMember(type, ZarrDatatype.MATLABTypes);
end
And then use
MATLABType (1,1) string {mustBeTextScalar, mustBeMATLABType}
| classdef ZarrDatatype | ||
| %ZARRDATATYPE Datatype of Zarr data | ||
| % Represents the datatype mapping between MATLAB, Tensorstore, and Zarr | ||
|
|
…for unsupported datatype in tZarrCreate
Yup, ran the tests when creating the PR and now with the new changes too! |
Move the logic of mapping between datatypes into a separate class, ZarrDatatype (instead of using dictionaries in Zarr class)
Fixes #14