search

uchicago-cs / deepdish

Flexible HDF5 saving/loading and other data science tools from the University of Chicago
http://deepdish.io
BSD 3-Clause "New" or "Revised" License
270 stars 59 forks source link

Resolution to issue #14: add 'mode' option to save() #15

Closed gaow closed 8 years ago

gaow commented 8 years ago

This patch addresses to issue https://github.com/uchicago-cs/deepdish/issues/14.

gustavla commented 8 years ago

This seems really useful and I really like the idea. However, I am reluctant to merge this just yet for two reasons:

  1. I'm not sure if I like mode='a', as opposed to something like append=True. Mode to me is something you specify when you open a file handler, but calling dd.io.save doesn't "open" a file handler, since save opens and closes it (which is essentially an implementation detail that the user of save should not even be exposed to). It also exposes too much of the internals - let's say save gets another backend one day, in which mode is not used. Then we would have to start parsing the mode and replicate the semantics. Which leads me to...
  2. The semantics are not what you would expect. You can currently only append or update root groups. If I want to update or add to a subgroup, I get an error message. I think I'd like to hold off on introducing an append feature until you can append and update a group of any level.

I will close this for now, but it's definitely a very useful feature that I would like to see in deepdish some day.