Adding attrs arg to create_group and create_dataset

[jakirkham]

As it stands now, when one writes a group or dataset, one must write the attributes as a separate operation. This results in creation of .zattrs once. Then if one update the attributes, it results in writing .zattrs again. With something like a ZipStore backed Zarr Group, one gets a warning on writing .zattrs the second time, which AFAICT is unavoidable unless someone sets the attributes correctly the first time. So if we include an argument for attrs to create_group and create_dataset, we should be able to avoid this warning by setting the attributes during creation of these Zarr objects.

[alimanfoo]

This is a nice idea, I'd be happy for an 'attrs' keyword argument to be added to all array and group creation functions.

…

On Wednesday, February 22, 2017, jakirkham ***@***.***> wrote: As it stands now, when one writes a group or dataset, one must write the attributes as a separate operation. This results in creation of .zattrs once. Then if one update the attributes, it results in writing .zattrs again. With something like a ZipStore backed Zarr Group, one gets a warning on writing .zattrs the second time, which AFAICT is unavoidable unless someone sets the attributes correctly the first time. So if we include an argument for attrs to create_group and create_dataset, we should be able to avoid this warning by setting the attributes during creation of these Zarr objects. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <https://github.com/alimanfoo/zarr/issues/121>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAq8Qou7tQVLcrCXQIjNimzufpA8U3xeks5rfHoAgaJpZM4MI_sc> .

-- Alistair Miles Head of Epidemiological Informatics Centre for Genomics and Global Health <http://cggh.org> The Wellcome Trust Centre for Human Genetics Roosevelt Drive Oxford OX3 7BN United Kingdom Email: [email protected] Web: http://purl.org/net/aliman Twitter: https://twitter.com/alimanfoo Tel: +44 (0)1865 287721

[jakirkham]

Would also add open_group to this list. Though I guess this function raises a question about how to handle cases where the group already exists and what to do if these attrs match or don't.

[alimanfoo]

Could just update any existing attributes?

On Wed, 18 Oct 2017 at 18:41, jakirkham ***@***.***> wrote: Would also add open_group to this list. Though I guess this function raises a question about how to handle cases where the group already exists and what to do if these attrs match or don't. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <https://github.com/alimanfoo/zarr/issues/121#issuecomment-337670143>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAq8QqpJ46BsZRgGoip_n-Kt7AYBJjx0ks5stjgAgaJpZM4MI_sc> .

-- Alistair Miles Head of Epidemiological Informatics Centre for Genomics and Global Health <http://cggh.org> Big Data Institute Building Old Road Campus Roosevelt Drive Oxford OX3 7LF United Kingdom Phone: +44 (0)1865 743596 Email: [email protected] Web: http://a <http://purl.org/net/aliman>limanfoo.github.io/ Twitter: https://twitter.com/alimanfoo

[jakirkham]

If one makes any changes to the attributes, it will be added as a new entry in the Zip file, which is exactly what I would like to avoid.

[alimanfoo]

Ah OK. Maybe attrs arg is ignored then if array of group already exists?

On Wed, 18 Oct 2017 at 23:15, jakirkham ***@***.***> wrote: If one makes any changes to the attributes, it will be added as a new entry in the Zip file, which is exactly what I would like to avoid. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <https://github.com/alimanfoo/zarr/issues/121#issuecomment-337743845>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAq8Qo-QrHnsKlnTWqhRa6rR4crX8RqUks5stniRgaJpZM4MI_sc> .

-- Alistair Miles Head of Epidemiological Informatics Centre for Genomics and Global Health <http://cggh.org> Big Data Institute Building Old Road Campus Roosevelt Drive Oxford OX3 7LF United Kingdom Phone: +44 (0)1865 743596 Email: [email protected] Web: http://a <http://purl.org/net/aliman>limanfoo.github.io/ Twitter: https://twitter.com/alimanfoo

[jakirkham]

Maybe that would be ok if we check that they don't differ first. Wouldn't want to lose information here. Alternatively we could just start adding copy_from_* style-methods, which would be generally useful and maybe less kludgy than my original suggestion.

In any event, had to switch to using the zipfile module directly to get tighter control on memory consumption and squeeze out every last bit of efficiency possible. As such this is not a pressing feature for me personally (though may be nice for others). Am happy to let it sit or close as deemed appropriate.

[jakirkham]

Honestly I'd be ok closing this one. While attrs certainly could be an optional argument to functions creating datasets and groups, it diverges from h5py a bit. I think the problem that I had can be solved better ways. Not to mention, the copy suggestion I made above ends up being the same as issue ( https://github.com/alimanfoo/zarr/issues/113 ). Having copy would be of significantly more utility than optionally adding attrs. Thoughts?

[alimanfoo]

Thinking about this some more, I actually think the current behaviour is a bit pointless: when an array or group is created, an empty dict is written to the .zattrs key, but there is no need to write .zattrs at all unless the user actually wants to store some attributes. I'm tempted to change this so that .zattrs is only written when the user actually wants to store some attributes. This would obviate the need for an 'attrs' argument to array or group creation, because, e.g.:

g = zarr.group()
g.attrs.update(foo='bar', baz='qux')

...would then imply only a single write to .zattrs.

[jakirkham]

Yeah I like that idea better as well. Does it mean a change in the spec though? Not sure if .zattrs was guaranteed to always be there or not.

[alimanfoo]

Just looking at the spec now, actually it never said that .zattrs was required to be there, so I think we can just clarify that .zattrs is optional and if absent means that attributes should be treated as empty.

…

On Fri, Nov 24, 2017 at 1:07 AM, jakirkham ***@***.***> wrote: Yeah I like that idea better as well. Does it mean a change in the spec though? Not sure if .zattrs was guaranteed to always be there or not. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <https://github.com/alimanfoo/zarr/issues/121#issuecomment-346716108>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAq8Qt6ZA3rZTCnflOagHduiRmnBNRpCks5s5hbdgaJpZM4MI_sc> .

-- Alistair Miles Head of Epidemiological Informatics Centre for Genomics and Global Health <http://cggh.org> Big Data Institute Building Old Road Campus Roosevelt Drive Oxford OX3 7LF United Kingdom Phone: +44 (0)1865 743596 Email: [email protected] Web: http://a <http://purl.org/net/aliman>limanfoo.github.io/ Twitter: https://twitter.com/alimanfoo

[alimanfoo]

I've gone ahead and implemented this in #200, everything works fine and should be fully backwards compatible with code and data, so seems like a good thing to me.