Centralize encoding versions and test them #289
Conversation
| @@ -191,7 +192,7 @@ def write_csr(f, key, value, dataset_kwargs=MappingProxyType({})): | |||
| def write_csc(f, key, value, dataset_kwargs=MappingProxyType({})): | |||
| group = f.create_group(key) | |||
| group.attrs["encoding-type"] = "csc_matrix" | |||
| group.attrs["enocding-version"] = "0.1.0" | |||
| group.attrs["encoding-version"] = EncodingVersions.csc_matrix.value | |||
There was a problem hiding this comment.
Hey @falexwolf @ivirshup: This is why I’m so anal about reusing code.
You made that typo only in one of the variants, only in zarr. Copy&paste isn’t better because as soon as one of the copies get edited, the others desync.
There was a problem hiding this comment.
@flying-sheep, my main idea was for them not to be in sync. This way we don't need to update every elements versions when only one has changed.
An example of this is I would like to update just the csc and csr matrices to make sure they only store sorted indices (which would make offline access faster). In this case, nothing about the dataframe encoding would require updating.
This also applies to zarr and hdf5, features can differ between them, and I don't think we need to update both versions if we find a bug that only relates to one.
There was a problem hiding this comment.
What do you mean? they aren’t in sync. Each value gets read from a different enum member. Did you read the code?
You’re right about zarr and HDF5 though, those are currently the same. In case we ever want to bump one and not the other, we can just split the enum into two.
Since we have versions we should also check them. Fixes #287