MAINT: update file extension logic#962
Conversation
ColmTalbot
left a comment
ColmTalbot
left a comment
There was a problem hiding this comment.
This generally looks good. On testing, it would be good to add some tests of what happens if you attempt to read a file saved with one format with the wrong read_in_result extension.
| ) | ||
| logger.warning(message) | ||
|
|
||
| if extension is None: |
There was a problem hiding this comment.
Since we're adding logging, it would be good to add an info like "No file extension specified, defaulting to JSON."
There was a problem hiding this comment.
Good point, I've added a couple more statements.
|
Also, I've seen the precommit failure recently, I'm not really sure where it comes from and why main isn't failing. |
Good idea. Whilst adding these tests I realised it would probably be insightful to actually raise a helpful error, so I've slightly reworked the error handling in |
I see you've fixed that in #965, hopefully we can this in once that's merged. |
mj-will
force-pushed
the
fix-extensions
branch
from
151405c to
3648887
Compare
* MAINT: rework file extension logic * TST: update result saving tests * BUG: fix extension precedence in `read_in_result` * TST: assert files can be read in * MAINT: tweak extension logic in `read_in_result` * TST: update extensions in tests * BUG: fix extension default * BUG: fix logic in `read_in_result` * TST: improve result save/read tests * ENH: improve error handling in `read_in_result` * TST: add tests for file loading with incorrect extensions * MAINT: tweak IOError message * TST: fix logging check in caching test --------- Co-authored-by: Colm Talbot <talbotcolm@gmail.com>
* MAINT: rework file extension logic * TST: update result saving tests * BUG: fix extension precedence in `read_in_result` * TST: assert files can be read in * MAINT: tweak extension logic in `read_in_result` * TST: update extensions in tests * BUG: fix extension default * BUG: fix logic in `read_in_result` * TST: improve result save/read tests * ENH: improve error handling in `read_in_result` * TST: add tests for file loading with incorrect extensions * MAINT: tweak IOError message * TST: fix logging check in caching test --------- Co-authored-by: Colm Talbot <talbotcolm@gmail.com>
* MAINT: rework file extension logic * TST: update result saving tests * BUG: fix extension precedence in `read_in_result` * TST: assert files can be read in * MAINT: tweak extension logic in `read_in_result` * TST: update extensions in tests * BUG: fix extension default * BUG: fix logic in `read_in_result` * TST: improve result save/read tests * ENH: improve error handling in `read_in_result` * TST: add tests for file loading with incorrect extensions * MAINT: tweak IOError message * TST: fix logging check in caching test --------- Co-authored-by: Colm Talbot <talbotcolm@gmail.com>
3 participants
Update the file extension logic in
Result.save_to_fileto match what is described in the doc-string.The main change is to no longer replace or add an extension when
filenameis specified. For examplefilename=test, extension='pkl'will just save totestrather thantest.pkl.I also update
read_in_resultso that extension isn't overridden.This consistent with versions <2.4.0 but differs to versions since.
I also added explicit support for
h5andpickle.Changes to
read_in_resultNone.json) if the filename is not provided. This means the behaviour whenfilename=Noneis identical. Whenfilenameis specified it is the behaviour is also the same but the error that is raised whenextension=Noneand the filename is missing in extension is slightly different.