H5py: Feature request: User control for argument default values

[ivirshup]

I think it would be nice if it was possible to do:

with h5py.File(pth, "w") as f, h5py.defaults(compression="lzf", chunks=True):
    f["X"] = X
    f["Y"] = Y
    with h5py.defaults(maxshape=None):
        f["foo/Z"] = Z

Instead of

with h5py.File(pth, "w") as f:
    f.create_dataset("X", data=X, compression="lzf", chunks=True)
    f.create_dataset("Y", data=Y, compression="lzf", chunks=True)
    f.create_dataset("foo/Z", data=Z, compression="lzf", chunks=True, maxshape=None)

The second is much more verbose, and makes it much easier to forget to pass some kwargs (which inspired this :blush:).

[takluyver]

I'm generally -1 on things like this. It's hidden state: the effect of a line of code depends on some other code that may not be on the same screen or the same file, and there's no variable name to track down for it. I'd rather have to pass these options directly, even if it makes the code more verbose.

There's this simple option with plain Python - not as short as what you suggest, but preserving explicitness with a variable you can trace back:

ds_opts = {'compression': 'lzf', 'chunks': True}
f.create_dataset('X', data=X, **ds_opts)

And of course you're free to build your own higher-level API on top of h5py if you want something more convenient.

[ivirshup]

The main reason I would like this (or a way to set defaults on a group) would be to make the MutableMapping interface more useful. Right now, using this interface locks you into uncompressed storage. I think this interface could be much more composable if there was a way for users to control (at least) dataset creation options.

For instance, awkward array provides a function for writing their data to an arbitrary hierarchical array store, ak.to_arrayset. Here's an example using h5py from the awkward array docs. Since it's using the mutable mapping interface, options for compressing the data are limited to writing a wrapper class or writing a different method for saving the array.

I would concede that it doesn't have to be context manager, but a way to choose values for keyword arguments while using the MutableMapping interface would make that interface much more useful.

---

As for the hidden state argument, I'm not sure passing **kwargs is that much more traceable than using a context manager. I don't feel too strongly about this, but I think there some notable downsides to passing around a dict as well (mutability, keys/ values don't get validated until create_dataset is called).

Also, a side goal is to reduce the amount of times I have to type: datatset_kwargs: Mapping = MappingProxyType({}). Right now, it's a lot. In this file, the dataset_kwargs variable is used 80 times. This could be reduced, but I don't think by that much.

[takluyver]

I'm not sure passing **kwargs is that much more traceable than using a context manager.

What I mean by traceability is that there's a variable name that you can look back to see where it comes from. With a context manager, you can call a function inside the context manager, and it will affect code inside the function, but there's no visible sign of it in the function's code.

How about, instead of a context manager, a wrapper class that exposes the mapping interface, so you can do something like this:

f = h5py.File(pth, "w") as f
f_lzf = GroupMapWrap(f, compression='lzf', chunks=True)
f_lzf['X'] = X
f_lzf["Y"] = Y
f_lzf_unlim = GroupMapWrap(f_lzf, maxshape=None)
f_lzf_unlim["foo/Z"] = Z

(I don't much like the name GroupMapWrap - better ideas welcome)

This would let you use the mapping interface to create datasets, but it avoids hidden state: a GroupMapWrap (or whatever it's called) can only affect code that has a reference to it - when you pass it into a function, store it on an object, etc.

It should also be possible to prototype this interface using only the public API of h5py, so you can try it out in your own code before we commit to it in h5py.

[tacaswell]

I see the API motivation from @ivirshup and think that would make the interface more "fluid" to use, but I also strongly agree with @takluyver about hidden global state being very confusing. I could be convinced on "both" with

with h5py.File(...) as f:
    with DefaultSetter(f, **kwargs) as nice_f:
        nice_f['bob'] = 5

(where the object nice_f renders itself unusable on context manager exit)

or maybe

with h5py.File(...) as f:
    with f.set_dset_defaults(**kwargs) as f:
        f['bob'] = 5
        g = f.ensure_group('bar') 
        with g.set_dset_defaults(**other_kwargs): as g:
            g['baz'] = [1, 2, 3]

(where the context manager returns the host object but resets the settings on context manager exit)

In either case, the hill I am willing to die on is that the state should be attached to the File/Group not global to h5py.

[ivirshup]

I'd agree with that! I like @tacaswell's solution of being able to set the defaults on the objects.

It took me a while to get back on this, but gave the GroupMapWrap a try (implementation below), and it was weird to have two MutableMapping interfaces to the same thing with slightly different APIs.

My only other pro a h5py level context manager argument: it would be easier to add options to code that doesn't expose h5py. This came from using mpl.rc_context, since that's been very useful for modifying aspects of figures generated by code I don't control. Of course, there are many more options in a plotting library than in a hdf5 interface.

GroupMapWrap implementation

from collections.abc import MutableMapping, Mapping
import h5py


class GroupMapWrap(MutableMapping):
    def __init__(self, group: h5py.Group, dataset_kwargs: Mapping = {}, group_kwargs: Mapping = {}):
        if isinstance(group, GroupMapWrap):
            # To allow progressivley adding options, would be nice to just dispatch on
            orig_dataset_kwargs = group.dataset_kwargs.copy()
            orig_dataset_kwargs.update(dataset_kwargs)
            dataset_kwargs = orig_dataset_kwargs
            orig_group_kwargs = group.group_kwargs.copy()
            orig_group_kwargs.update(group_kwargs)
            group_kwargs = orig_group_kwargs
            group = group.group
        self.group = group
        self.dataset_kwargs = dataset_kwargs
        self.group_kwargs = group_kwargs

    def __getitem__(self, key):
        return self.group[key]

    def __setitem__(self, key, value):
        if isinstance(value, Mapping):
            mapping = GroupMapWrap(
                self.group.create_group(key, **self.group_kwargs),
                dataset_kwargs=self.dataset_kwargs,
                group_kwargs=self.group_kwargs,
            )
            mapping.update(value)
        else:
            self.group.create_dataset(key, data=value, **self.dataset_kwargs)

    def __delitem__(self, key):
        del self.group[key]

    def __iter__(self):
        return iter(self.group)

    def __len__(self):
        return len(self.group)

[tacaswell]

since that's been very useful for modifying aspects of figures generated by code I don't control

That is exactly the spooky action-at-a-distance @takluyver is concerned about. My concern about global state for this sort of thing is driven by being on the developer side of mpl.rcParams!

I think the order of operations here:

1. add a 'default_group_kwargs' and default_ds_kwargs state to h5py._hl.group.Group object. These should propogate down to sub-groups opened / created from their parent.
2. add a context manager to Group to set / unset defaults
3. make the case that File should consult global state to get its initial default values

I think 1. addresses most of @ivirshup initial request using only "local" state. So long as you can pass an h5py object into the third-party code it also catches that case (which I hope is most but I know not all casse). 2. is a nice API (and light weight to implement). 3. is the action-at-a-distance that I see the case for, but do not like (would rather ask you to push on the third party code to either take a Group or at least kwargs to pass to File along with the filename to make use of 1.)

[tacaswell]

On a bit more thought, I remembered we do already have a global config (https://github.com/h5py/h5py/blob/95e3497e896e49192a6e46521fb7b9d4d526cf92/h5py/h5.pyx#L169-L174 ) that stores a number of things including if the creation order should be tracked so step 3 is a much smaller step than I thought it would be.

[takluyver]

I'd prefer not to expand the use of the global config for this kind of thing. I don't think it's a particularly good API - but it smooths over things like how we represent types that HDF5 can't natively store (complex & bool) and changing default settings with a deprecation cycle. I'm not convinced it's a good idea for track_order, and I don't think the fact that we already have some global mutable state should set a precedent that we should have more.

I'd still like to make the case for doing this with some flavour of the GroupMapWrap or DefaultSetter idea, without using any extra mutable state. @ivirshup , can you go into more detail about what was 'weird' when you tried this?