From 2e777204469f4574e7baeb4b515e90f121af76d6 Mon Sep 17 00:00:00 2001 From: Pradyun Gedam Date: Tue, 29 Jan 2019 14:39:02 +0530 Subject: [PATCH 1/5] Mention gone_in version in deprecation messages --- src/pip/_internal/utils/deprecation.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/pip/_internal/utils/deprecation.py b/src/pip/_internal/utils/deprecation.py index 8c896f8ce..b43b98db3 100644 --- a/src/pip/_internal/utils/deprecation.py +++ b/src/pip/_internal/utils/deprecation.py @@ -81,6 +81,11 @@ def deprecated(reason, replacement, gone_in, issue=None): # This is purposely eagerly formatted as we want it to appear as if someone # typed this entire message out. message = DEPRECATION_MSG_PREFIX + reason + + if gone_in is not None: + message += ( + " pip {} will remove support for this functionality".format(gone_in) + ) if replacement is not None: message += " A possible replacement is {}.".format(replacement) if issue is not None: From 995ef743ec99b91ff1895511dcc77f1ca722c38f Mon Sep 17 00:00:00 2001 From: Pradyun Gedam Date: Sat, 25 May 2019 21:52:49 -0400 Subject: [PATCH 2/5] Add tests for the deprecated helper --- tests/unit/test_utils.py | 77 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 77 insertions(+) diff --git a/tests/unit/test_utils.py b/tests/unit/test_utils.py index 1d97e65a1..3e9087fb5 100644 --- a/tests/unit/test_utils.py +++ b/tests/unit/test_utils.py @@ -22,6 +22,7 @@ from mock import Mock, patch from pip._internal.exceptions import ( HashMismatch, HashMissing, InstallationError, ) +from pip._internal.utils.deprecation import PipDeprecationWarning, deprecated from pip._internal.utils.encoding import BOMS, auto_decode from pip._internal.utils.glibc import check_glibc_version from pip._internal.utils.hashes import Hashes, MissingHashes @@ -1068,3 +1069,79 @@ def test_remove_auth_from_url(auth_url, expected_url): def test_redact_password_from_url(auth_url, expected_url): url = redact_password_from_url(auth_url) assert url == expected_url + + +@pytest.fixture() +def patch_deprecation_check_version(): + # We do this, so that the deprecation tests are easier to write. + import pip._internal.utils.deprecation as d + old_version = d.current_version + d.current_version = "1.0" + yield + d.current_version = old_version + + +@pytest.mark.usefixtures("patch_deprecation_check_version") +@pytest.mark.parametrize("replacement", [None, "a magic 8 ball"]) +@pytest.mark.parametrize("gone_in", [None, "2.0"]) +@pytest.mark.parametrize("issue", [None, 988]) +def test_deprecated_message_contains_information(gone_in, replacement, issue): + with pytest.warns(PipDeprecationWarning) as record: + deprecated( + "Stop doing this!", + replacement=replacement, + gone_in=gone_in, + issue=issue, + ) + + assert len(record) == 1 + message = record[0].message.args[0] + + assert "DEPRECATION: Stop doing this!" in message + # Ensure non-None values are mentioned. + for item in [gone_in, replacement, issue]: + if item is not None: + assert str(item) in message + + +@pytest.mark.usefixtures("patch_deprecation_check_version") +@pytest.mark.parametrize("replacement", [None, "a magic 8 ball"]) +@pytest.mark.parametrize("issue", [None, 988]) +def test_deprecated_raises_error_if_too_old(replacement, issue): + with pytest.raises(PipDeprecationWarning) as exception: + deprecated( + "Stop doing this!", + gone_in="1.0", # this matches the patched version. + replacement=replacement, + issue=issue, + ) + + message = exception.value.args[0] + + assert "DEPRECATION: Stop doing this!" in message + assert "1.0" in message + # Ensure non-None values are mentioned. + for item in [replacement, issue]: + if item is not None: + assert str(item) in message + + +@pytest.mark.usefixtures("patch_deprecation_check_version") +def test_deprecated_message_reads_well(): + with pytest.raises(PipDeprecationWarning) as exception: + deprecated( + "Stop doing this!", + gone_in="1.0", # this matches the patched version. + replacement="to be nicer", + issue="100000", # I hope we never reach this number. + ) + + message = exception.value.args[0] + + assert message == ( + "DEPRECATION: Stop doing this! " + "pip 1.0 will remove support for this functionality. " + "A possible replacement is to be nicer. " + "You can find discussion regarding this at " + "https://github.com/pypa/pip/issues/100000." + ) From bf728499beee85947d7c29a94ee8f4983d4a08a0 Mon Sep 17 00:00:00 2001 From: Pradyun Gedam Date: Sat, 25 May 2019 21:56:20 -0400 Subject: [PATCH 3/5] :art: --- src/pip/_internal/utils/deprecation.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/pip/_internal/utils/deprecation.py b/src/pip/_internal/utils/deprecation.py index b43b98db3..ed07eb5f0 100644 --- a/src/pip/_internal/utils/deprecation.py +++ b/src/pip/_internal/utils/deprecation.py @@ -78,8 +78,8 @@ def deprecated(reason, replacement, gone_in, issue=None): """ # Construct a nice message. - # This is purposely eagerly formatted as we want it to appear as if someone - # typed this entire message out. + # This is eagerly formatted as we want it to get logged as if someone + # typed this entire message out. message = DEPRECATION_MSG_PREFIX + reason if gone_in is not None: @@ -95,4 +95,5 @@ def deprecated(reason, replacement, gone_in, issue=None): # Raise as an error if it has to be removed. if gone_in is not None and parse(current_version) >= parse(gone_in): raise PipDeprecationWarning(message) + warnings.warn(message, category=PipDeprecationWarning, stacklevel=2) From 2ff13e4340f263122d429dbf957297b64657222f Mon Sep 17 00:00:00 2001 From: Pradyun Gedam Date: Sun, 26 May 2019 08:21:51 -0400 Subject: [PATCH 4/5] Use a loop instead of multiple similar conditionals --- src/pip/_internal/utils/deprecation.py | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/src/pip/_internal/utils/deprecation.py b/src/pip/_internal/utils/deprecation.py index ed07eb5f0..b9359bddc 100644 --- a/src/pip/_internal/utils/deprecation.py +++ b/src/pip/_internal/utils/deprecation.py @@ -80,17 +80,18 @@ def deprecated(reason, replacement, gone_in, issue=None): # Construct a nice message. # This is eagerly formatted as we want it to get logged as if someone # typed this entire message out. - message = DEPRECATION_MSG_PREFIX + reason - - if gone_in is not None: - message += ( - " pip {} will remove support for this functionality".format(gone_in) - ) - if replacement is not None: - message += " A possible replacement is {}.".format(replacement) - if issue is not None: - url = "https://github.com/pypa/pip/issues/" + str(issue) - message += " You can find discussion regarding this at {}.".format(url) + sentences = [ + (reason, DEPRECATION_MSG_PREFIX + "{}"), + (gone_in, "pip {} will remove support for this functionality."), + (replacement, "A possible replacement is {}."), + (issue, ( + "You can find discussion regarding this at " + "https://github.com/pypa/pip/issues/{}." + )), + ] + message = " ".join( + template.format(val) for val, template in sentences if val is not None + ) # Raise as an error if it has to be removed. if gone_in is not None and parse(current_version) >= parse(gone_in): From 5802a63799495a0142158b0b1b36be8158c3b130 Mon Sep 17 00:00:00 2001 From: Pradyun Gedam Date: Mon, 27 May 2019 13:59:48 -0400 Subject: [PATCH 5/5] :newspaper: --- news/6549.feature | 1 + 1 file changed, 1 insertion(+) create mode 100644 news/6549.feature diff --git a/news/6549.feature b/news/6549.feature new file mode 100644 index 000000000..d4970e39a --- /dev/null +++ b/news/6549.feature @@ -0,0 +1 @@ +Improve deprecation messages to include the version in which the functionality will be removed.