From 445a575bc439d146338dd9210e152b41cdff4db0 Mon Sep 17 00:00:00 2001 From: Thomas Nicholas Date: Tue, 11 May 2021 11:10:50 -0400 Subject: [PATCH 1/9] explained what a deprecation cycle is --- doc/contributing.rst | 24 +++++++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) diff --git a/doc/contributing.rst b/doc/contributing.rst index e10ceacd59f..ec0edfbc5c6 100644 --- a/doc/contributing.rst +++ b/doc/contributing.rst @@ -379,10 +379,28 @@ with ``git commit --no-verify``. Backwards Compatibility ~~~~~~~~~~~~~~~~~~~~~~~ -Please try to maintain backward compatibility. *xarray* has growing number of users with +Please try to maintain backwards compatibility. *xarray* has growing number of users with lots of existing code, so don't break it if at all possible. If you think breakage is -required, clearly state why as part of the pull request. Also, be careful when changing -method signatures and add deprecation warnings where needed. +required, clearly state why as part of the pull request. + +Be especially careful when changing function and method signatures, because any change +may require a deprecation warning. For example, if your pull request means that the +argument `old_arg` to `func` is no longer valid, instead of simply raising an error if +a user passes `old_arg`, we would instead catch it: + + def func(new_arg, old_arg=None): + if old_arg is not None: + from warnings import warn + warn("`old_arg` has now been deprecated, and in future will raise an error. + please use `new_arg` from now on.") + + # Still do what the user intended here + +This temporary check would then be removed in the subsequent version of xarray. +This process of first warning users before actually breaking their code is known as a +"deprecation cycle", and makes changes significantly easier to handle both for users +of xarray, and for developers of other libraries that depend on xarray. + .. _contributing.ci: From bdf507f03dc7a3a6a556c2701b6b05b52f0b763c Mon Sep 17 00:00:00 2001 From: Thomas Nicholas Date: Tue, 11 May 2021 11:16:08 -0400 Subject: [PATCH 2/9] Capitalization [no-ci] --- doc/contributing.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/contributing.rst b/doc/contributing.rst index ec0edfbc5c6..cf54ec5ae61 100644 --- a/doc/contributing.rst +++ b/doc/contributing.rst @@ -392,7 +392,7 @@ a user passes `old_arg`, we would instead catch it: if old_arg is not None: from warnings import warn warn("`old_arg` has now been deprecated, and in future will raise an error. - please use `new_arg` from now on.") + Please use `new_arg` from now on.") # Still do what the user intended here From a9db4064aa7d1460352f0f068229807c59161303 Mon Sep 17 00:00:00 2001 From: Tom Nicholas <35968931+TomNicholas@users.noreply.github.com> Date: Tue, 11 May 2021 11:33:40 -0400 Subject: [PATCH 3/9] grammar [no-ci] Co-authored-by: Anderson Banihirwe --- doc/contributing.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/contributing.rst b/doc/contributing.rst index cf54ec5ae61..ea6e5b0ca8f 100644 --- a/doc/contributing.rst +++ b/doc/contributing.rst @@ -379,7 +379,7 @@ with ``git commit --no-verify``. Backwards Compatibility ~~~~~~~~~~~~~~~~~~~~~~~ -Please try to maintain backwards compatibility. *xarray* has growing number of users with +Please try to maintain backwards compatibility. *xarray* has a growing number of users with lots of existing code, so don't break it if at all possible. If you think breakage is required, clearly state why as part of the pull request. From ddf8034b0fb071da4adafb826c9a983f1e26ed6b Mon Sep 17 00:00:00 2001 From: Tom Nicholas <35968931+TomNicholas@users.noreply.github.com> Date: Tue, 11 May 2021 12:12:47 -0400 Subject: [PATCH 4/9] Split warning across lines Co-authored-by: Mathias Hauser --- doc/contributing.rst | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/doc/contributing.rst b/doc/contributing.rst index ea6e5b0ca8f..dec8d2eb095 100644 --- a/doc/contributing.rst +++ b/doc/contributing.rst @@ -391,8 +391,11 @@ a user passes `old_arg`, we would instead catch it: def func(new_arg, old_arg=None): if old_arg is not None: from warnings import warn - warn("`old_arg` has now been deprecated, and in future will raise an error. - Please use `new_arg` from now on.") + + warn( + "`old_arg` has been deprecated, and in the future will raise an error." + "Please use `new_arg` from now on." + ) # Still do what the user intended here From c903ba8b49e33dca4e418f39f2ce0a6f54e9718d Mon Sep 17 00:00:00 2001 From: Tom Nicholas <35968931+TomNicholas@users.noreply.github.com> Date: Tue, 11 May 2021 12:13:13 -0400 Subject: [PATCH 5/9] the -> a Co-authored-by: Mathias Hauser --- doc/contributing.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/contributing.rst b/doc/contributing.rst index dec8d2eb095..aa973332a6e 100644 --- a/doc/contributing.rst +++ b/doc/contributing.rst @@ -399,7 +399,7 @@ a user passes `old_arg`, we would instead catch it: # Still do what the user intended here -This temporary check would then be removed in the subsequent version of xarray. +This temporary check would then be removed in a subsequent version of xarray. This process of first warning users before actually breaking their code is known as a "deprecation cycle", and makes changes significantly easier to handle both for users of xarray, and for developers of other libraries that depend on xarray. From 77cfc8e146f61d4033155d8701bd34a7613f0fbe Mon Sep 17 00:00:00 2001 From: Tom Nicholas <35968931+TomNicholas@users.noreply.github.com> Date: Tue, 11 May 2021 12:13:43 -0400 Subject: [PATCH 6/9] Proper quotes around methods [skip-ci] Co-authored-by: Mathias Hauser --- doc/contributing.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/contributing.rst b/doc/contributing.rst index aa973332a6e..532121c7218 100644 --- a/doc/contributing.rst +++ b/doc/contributing.rst @@ -385,8 +385,8 @@ required, clearly state why as part of the pull request. Be especially careful when changing function and method signatures, because any change may require a deprecation warning. For example, if your pull request means that the -argument `old_arg` to `func` is no longer valid, instead of simply raising an error if -a user passes `old_arg`, we would instead catch it: +argument ``old_arg`` to ``func`` is no longer valid, instead of simply raising an error if +a user passes ``old_arg``, we would instead catch it: def func(new_arg, old_arg=None): if old_arg is not None: From 738460873db4f65fd10bd71c76541bd7bafa63d9 Mon Sep 17 00:00:00 2001 From: Thomas Nicholas Date: Wed, 12 May 2021 11:15:46 -0400 Subject: [PATCH 7/9] added warning type to deprecation warning example --- doc/contributing.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/contributing.rst b/doc/contributing.rst index 532121c7218..9a64f929eb7 100644 --- a/doc/contributing.rst +++ b/doc/contributing.rst @@ -394,7 +394,7 @@ a user passes ``old_arg``, we would instead catch it: warn( "`old_arg` has been deprecated, and in the future will raise an error." - "Please use `new_arg` from now on." + "Please use `new_arg` from now on.", DeprecationWarning ) # Still do what the user intended here From 76f1696d4aacb90982eb6904775519d32e7e6e63 Mon Sep 17 00:00:00 2001 From: Thomas Nicholas Date: Wed, 12 May 2021 11:18:53 -0400 Subject: [PATCH 8/9] whats new [skip-ci] --- doc/whats-new.rst | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/doc/whats-new.rst b/doc/whats-new.rst index 3f81678b8d5..b235bf9f235 100644 --- a/doc/whats-new.rst +++ b/doc/whats-new.rst @@ -38,6 +38,10 @@ Bug fixes Documentation ~~~~~~~~~~~~~ +- Explanation of deprecation cycles and how to implement them added to contributors + guide. (:pull:`5289`) + By `Tom Nicholas `_. + Internal Changes ~~~~~~~~~~~~~~~~ From 67f0951fa392865c0f14ac4cfd3ea340259a60ec Mon Sep 17 00:00:00 2001 From: Thomas Nicholas Date: Wed, 12 May 2021 16:49:05 -0400 Subject: [PATCH 9/9] make example a code block --- doc/contributing.rst | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/doc/contributing.rst b/doc/contributing.rst index 9a64f929eb7..f43fc3e312c 100644 --- a/doc/contributing.rst +++ b/doc/contributing.rst @@ -388,13 +388,16 @@ may require a deprecation warning. For example, if your pull request means that argument ``old_arg`` to ``func`` is no longer valid, instead of simply raising an error if a user passes ``old_arg``, we would instead catch it: +.. code-block:: python + def func(new_arg, old_arg=None): if old_arg is not None: from warnings import warn warn( "`old_arg` has been deprecated, and in the future will raise an error." - "Please use `new_arg` from now on.", DeprecationWarning + "Please use `new_arg` from now on.", + DeprecationWarning, ) # Still do what the user intended here