Repository navigation
Adding attrs arg to create_group and create_dataset #121
Description
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.
Activity
alimanfoo commentedon Feb 22, 2017
jakirkham commentedon Oct 18, 2017
Would also add
open_groupto 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 theseattrsmatch or don't.alimanfoo commentedon Oct 18, 2017
jakirkham commentedon Oct 18, 2017
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 commentedon Oct 19, 2017
jakirkham commentedon Oct 23, 2017
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
zipfilemodule 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 commentedon Nov 20, 2017
Honestly I'd be ok closing this one. While
attrscertainly could be an optional argument to functions creating datasets and groups, it diverges fromh5pya bit. I think the problem that I had can be solved better ways. Not to mention, thecopysuggestion I made above ends up being the same as issue ( https://github.com/alimanfoo/zarr/issues/113 ). Havingcopywould be of significantly more utility than optionally addingattrs. Thoughts?alimanfoo commentedon Nov 24, 2017
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.:
...would then imply only a single write to .zattrs.
jakirkham commentedon Nov 24, 2017
Yeah I like that idea better as well. Does it mean a change in the spec though? Not sure if
.zattrswas guaranteed to always be there or not.alimanfoo commentedon Nov 24, 2017
alimanfoo commentedon Nov 24, 2017
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.