Normalize from_xyz functions #490
Conversation
arviz/data/io_netcdf.py
Outdated
| @@ -39,3 +39,7 @@ def save_data(data, filename, *, group="posterior", coords=None, dims=None): | |||
| """ | |||
| inference_data = convert_to_inference_data(data, group=group, coords=coords, dims=dims) | |||
| return inference_data.to_netcdf(filename) | |||
|
|
|||
|
|
|||
| from_netcdf = load_data # pylint: disable=invalid-name | |||
There was a problem hiding this comment.
Can we just rename the functions rather than alias them and have two of the same function in the API?
There was a problem hiding this comment.
We could. Then we only need to mention this change in "what's changed". Or deprecate old functionality.
There was a problem hiding this comment.
I vote for deprecate old functionality. If we want to be safe we can wrap load_data and add a warning
There was a problem hiding this comment.
i'm fine with deprecation, or using the fact that we're not 1.0 yet to just change it (starting up a RELEASE_NOTES.md is a good idea!)
| @@ -10,7 +10,7 @@ class PyMC3Converter: | |||
| """Encapsulate PyMC3 specific logic.""" | |||
|
|
|||
| def __init__( | |||
| self, *_, trace=None, prior=None, posterior_predictive=None, coords=None, dims=None | |||
| self, *, trace=None, prior=None, posterior_predictive=None, coords=None, dims=None | |||
There was a problem hiding this comment.
thank you for catching this :/
arviz/data/io_netcdf.py
Outdated
| @@ -39,3 +39,7 @@ def save_data(data, filename, *, group="posterior", coords=None, dims=None): | |||
| """ | |||
| inference_data = convert_to_inference_data(data, group=group, coords=coords, dims=dims) | |||
| return inference_data.to_netcdf(filename) | |||
|
|
|||
|
|
|||
| from_netcdf = load_data # pylint: disable=invalid-name | |||
There was a problem hiding this comment.
i'm fine with deprecation, or using the fact that we're not 1.0 yet to just change it (starting up a RELEASE_NOTES.md is a good idea!)
|
Based off the comments above I'm strongly in favor of
I know we're not in 1.0 release, but we should to replace globally anyway, and it would be nice to warn out existing users, including us, that we need to stop using I do realize global replacement is painful but I'm happy to do it, if we can wait a couple of weeks. I'm also fine merging this PR and then I follow up with my suggestions in another PR if that's what we choose. |
|
I can do it later today. |
|
This looks good to me! |
Use
kw1=None, *, kw2=None, ...in all of thefrom_xyzfunctions.Each library can still use their nomenclature if needed.
renamed
load_data(from_netcdf) andsave_data(to_netcdf). Old function names are now wrappers for their new name, and raise DeprecationWarning if used.