np.float64 vs float when compression is on.

[r-xue]

This was back to the discussion at the bottom of #136.
I just found out this...

test={'a':0.,'b':np.ones((1024,1024))}
hkl.dump(test, 'dummy.hkl', mode='w',compression='gzip')

is okay

test={'a':np.float64(0),'b':np.ones((1024,1024))}
hkl.dump(test, 'dummy.hkl', mode='w',compression='gzip')

will fail.

Looks like loaders.load_builtins.create_scalar_dataset() has this line:

# Make sure 'compression' is not in kwargs
kwargs.pop('compression', None)

which I guess let it pass.
Should we do the same in loaders.load_numpy.create_np_scalar_dataset()? though I am not sure if there are side-effects on this.

@1313e Thank you for any insight.

[1313e]

I knew there was something where I ignored it.
Although both implementations have their advantages and disadvantages, they should definitely be consistent.
I personally think we should remove that line in builtins, for the reason I gave in #136.

@telegraphic What's your opinion on this?

[telegraphic]

I vote to not pass any kwargs to create_dataset for numpy scalars, except track_times.

I agree it's a bit of a tough one. Compression is hugely useful and I think we should make every attempt to make it work for 'reasonable' cases. As mentioned, there are two straightforward solutions: we keep kwargs.pop, or don't pass kwargs.

Looking at the parameters for create_dataset:

Provide None to create an anonymous dataset, to be linked into the file later.
shape – Shape of new dataset (Tuple).
dtype – Data type for new dataset
data – Initialize dataset to this (NumPy array).
chunks – Chunk shape, or True to enable auto-chunking.
maxshape – Dataset will be resizable up to this shape (Tuple). Automatically enables chunking. Use None for the axes you want to be unlimited.
compression – Compression strategy. See Filter pipeline.
compression_opts – Parameters for compression filter.
scaleoffset – See Scale-Offset filter.
shuffle – Enable shuffle filter (T/F). See Shuffle filter.
fletcher32 – Enable Fletcher32 checksum (T/F). See Fletcher32 filter.
fillvalue – This value will be used when reading uninitialized parts of the dataset.
track_times – Enable dataset creation timestamps (T/F).
track_order – Track attribute creation order if True. Default is h5.get_config().track_order.
external – Store the dataset in one or more external, non-HDF5 files. This should be a list of tuples of (filename[, offset[, size]]), to store data from offset to offset + size in the specified file. The last file in the list may have size h5py.h5s.UNLIMITED to let it grow as needed.

A large number of these kwargs would cause TypeError: Scalar datasets don't support chunk/filter options for scalar datasets. Of these parameters, I think only track_times is supported/useful. So my vote is to only pass track_times, which is a more robust against other errant kwargs, and fewer lines to boot.

[hernot]

I also encountered the very same issue when working on pull-request #138 that compression can cause problems with scalars and strings. The latter up to now are stored as bytes with dtype='S<length>' for which compression is also not possible as for scalars.
This lead to several changes how code proposed by pull-request #138 handles scalars. For numerical scalars (builtin and numpy) and alike compression is temporarily deactivated. This is done decorating kwargs with no_compression generator.

Strings and byte strings are instead mapped to bytearray for which compression is accepted by h5py. The down of bytearray mapping is that they appear in hdf5 file as int8 or uint8 streams and not as readable char arrays. But given the fact that strings can be come quite long and may use plenty of space this should be a fair trade-of.

The no_compression decorating generator filters
compression, compression_opts, shuffle, fletcher_32, chunks, scaleoffset

# the kwargs are not kwargs to no_compression just
# a dictionary which needs the mentioned keys to be
# filtered
def no_compression(kwargs):
    return {
        key:value
        for key,value in kwargs.items()
        if key not in {"compression","shuffle","compression_opts","chunks","fletcher32","scaleoffset"}
    }

The follwowing would have to be added to set of options filtered by no_compression according to documentation pasted by @telegraphic :

maxshape as it implicitly sets chunks=True

That one is not listed by exception raised when compression is on and i also missed it form documentation.

I vote as opposit to you @telegraphic for filtering compression and chunking related keywords on scalars as proposed and convert strings and bytestrings to bytarray instead of disallowing comression of content completely.

Thus i do suggest to timely fix this disable comression globally through filtering in dump method and issuing a corresponding warning for now. A more sensitive solution would than used from 4.1.x versions on which might need some well thought design descision. The above sayed could serve as valuable input to design discussion.

[hernot]

Ok i think i have a solution or better some approach for rerunning all tests (at least those necessary) passing them with compression activated. Requires that systematic unit tests which are introduced in pull request #138 along with implementation of HEP001 #135. Will be included in finalize and clean-up pull request being the last of the in total four planned pull request. The first of which (#138) is currently under review. The other two will provide implementation of H4EP002 (#139) and H4EP003 (#145). Any thing else which can or has to be address by final polishing will be in the finalize and clean-up.

Ah and yes my tests already identified more yet unnoticed spots which need special handling of compression keyword arguments. Locally I already ammended them ;-)