Add support for PathLike objects to model save() and load() #1754
Conversation
sudrich
force-pushed
the
feature/add_pathlike_support
branch
from
a441c0e
to
3db5c9c
Compare
|
As mentioned in the associated issue, this is a duplicate of #1716. While scrolling through the changes, I noticed that the new lines in the changelog are not in the right section; you edited the release 0.24.0 instead of adding a new bullet point in the unreleased section. Can you please address this? |
My bad. Moved the entry to the correct section. |
Codecov ReportPatch coverage:
❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more. Additional details and impacted files@@ Coverage Diff @@
## master #1754 +/- ##
==========================================
- Coverage 94.18% 94.06% -0.12%
==========================================
Files 125 125
Lines 11500 11491 -9
==========================================
- Hits 10831 10809 -22
- Misses 669 682 +13
☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Thanks @sudrich, looks great 🚀
My only suggestion is to extend the test whether an error is raised for invalid paths (now that it should raise an error).
| @@ -154,8 +155,9 @@ def test_save_load_model(self): | |||
|
|
|||
| for model in [ARIMA(1, 1, 1), LinearRegressionModel(lags=12)]: | |||
| model_path_str = type(model).__name__ | |||
| model_path_file = model_path_str + "_file" | |||
| model_paths = [model_path_str, model_path_file] | |||
| model_path_pathlike = pathlib.Path(model_path_str + "_pathlike") | |||
There was a problem hiding this comment.
extending this test for invalid read/write path would be nice as well
There was a problem hiding this comment.
Done. I put it in an extra test though to avoid inflating the original test too much.
| # test save | ||
| self.assertRaises(ValueError, model.save, model_path_invalid) | ||
|
|
||
| # test load | ||
| self.assertRaises(ValueError, type(model).load, model_path_invalid) |
There was a problem hiding this comment.
| # test save | |
| self.assertRaises(ValueError, model.save, model_path_invalid) | |
| # test load | |
| self.assertRaises(ValueError, type(model).load, model_path_invalid) | |
| # test save | |
| with pytest.raises(ValueError): | |
| model.save(model_path_invalid) | |
| # test load | |
| with pytest.raises(ValueError): | |
| ARIMA.load(model_path_invalid) |
…1754) * Add support for PathLike to model save/load * Add entry to changelog * Use bufferedIO classes instead of BinaryIO for class check * Use raise_log for ValueErrors * Move entry to correct section of changelog * Add test for invalid inputs to save/load * Use pytest for checking ValueError --------- Co-authored-by: Dennis Bader <dennis.bader@gmx.ch>
Fixes #1751.
Summary
Added support for
PathLikeobjects to thesave()andload()functions ofForecastingModel.This is achieved by explicitly including
os.PathLikeas a possible input for the path.In addition, a check was added that raises a
ValueErrorfor invalid input arguments.Other Information