From 65931ce526615eb22199165d04c52ab3be3a9ea9 Mon Sep 17 00:00:00 2001 From: "Haoyu (Daniel)" Date: Fri, 3 Jan 2025 20:16:46 +0800 Subject: [PATCH 01/12] drop repo check and warn instead of raise --- src/monty/dev.py | 58 +++++++------------------------------ tests/test_dev.py | 74 +++++++++-------------------------------------- 2 files changed, 25 insertions(+), 107 deletions(-) diff --git a/src/monty/dev.py b/src/monty/dev.py index cf468bce4..320a022f7 100644 --- a/src/monty/dev.py +++ b/src/monty/dev.py @@ -8,8 +8,6 @@ import functools import inspect import logging -import os -import subprocess import sys import warnings from dataclasses import is_dataclass @@ -35,8 +33,8 @@ def deprecated( replacement (Callable | str): A replacement class or function. message (str): A warning message to be displayed. deadline (Optional[tuple[int, int, int]]): Optional deadline for removal - of the old function/class, in format (yyyy, MM, dd). A CI warning would - be raised after this date if is running in code owner' repo. + of the old function/class, in format (yyyy, MM, dd). DeprecationWarning + would be emitted after this date. category (Warning): Choose the category of the warning to issue. Defaults to FutureWarning. Another choice can be DeprecationWarning. Note that FutureWarning is meant for end users and is always shown unless silenced. @@ -48,45 +46,6 @@ def deprecated( Original function, but with a warning to use the updated function. """ - def raise_deadline_warning() -> None: - """Raise CI warning after removal deadline in code owner's repo.""" - - def _is_in_owner_repo() -> bool: - """Check if is running in code owner's repo. - Only generate reliable check when `git` is installed and remote name - is "origin". - """ - - try: - # Get current running repo - result = subprocess.run( - ["git", "config", "--get", "remote.origin.url"], - stdout=subprocess.PIPE, - ) - owner_repo = ( - result.stdout.decode("utf-8") - .strip() - .lstrip("https://github.com/") # HTTPS clone - .lstrip("git@github.com:") # SSH clone - .rstrip(".git") # SSH clone - ) - - return owner_repo == os.getenv("GITHUB_REPOSITORY") - - except (subprocess.CalledProcessError, FileNotFoundError): - return False - - # Only raise warning in code owner's repo CI - if ( - _deadline is not None - and os.getenv("CI") is not None - and datetime.now() > _deadline - and _is_in_owner_repo() - ): - raise DeprecationWarning( - f"This function should have been removed on {_deadline:%Y-%m-%d}." - ) - def craft_message( old: Callable, replacement: Callable | str, @@ -151,10 +110,15 @@ def new_init(self, *args, **kwargs): return cls # Convert deadline to datetime type - _deadline = datetime(*deadline) if deadline is not None else None - - # Raise CI warning after removal deadline - raise_deadline_warning() + _deadline: datetime | None = datetime(*deadline) if deadline is not None else None + + # Emit DeprecationWarning after removal deadline + if _deadline is not None and datetime.now() > _deadline: + warnings.warn( + f"This function should have been removed on {_deadline:%Y-%m-%d}.", + DeprecationWarning, + stacklevel=2, + ) def decorator(target: Callable) -> Callable: if inspect.isfunction(target): diff --git a/tests/test_dev.py b/tests/test_dev.py index 13bb211be..3451dd6dc 100644 --- a/tests/test_dev.py +++ b/tests/test_dev.py @@ -169,71 +169,25 @@ def method_b(self): assert old_class.__doc__ == "A dummy old class for tests." assert old_class.class_attrib_old == "OLD_ATTRIB" - def test_deprecated_deadline(self, monkeypatch): - with pytest.raises(DeprecationWarning): - with patch("subprocess.run") as mock_run: - monkeypatch.setenv("CI", "true") # mock CI env - - # Mock "GITHUB_REPOSITORY" - monkeypatch.setenv("GITHUB_REPOSITORY", "TESTOWNER/TESTREPO") - mock_run.return_value.stdout.decode.return_value = ( - "git@github.com:TESTOWNER/TESTREPO.git" - ) - - @deprecated(deadline=(2000, 1, 1)) - def func_old(): - pass - - @pytest.fixture() - def test_deprecated_deadline_no_warn(self, monkeypatch): - """Test cases where no warning should be raised.""" - - # No warn case 1: date before deadline - with warnings.catch_warnings(): - with patch("subprocess.run") as mock_run: - monkeypatch.setenv("CI", "true") # mock CI env - - # Mock date to 1999-01-01 - monkeypatch.setattr( - datetime.datetime, "now", datetime.datetime(1999, 1, 1) - ) - - # Mock "GITHUB_REPOSITORY" - monkeypatch.setenv("GITHUB_REPOSITORY", "TESTOWNER/TESTREPO") - mock_run.return_value.stdout.decode.return_value = ( - "git@github.com:TESTOWNER/TESTREPO.git" - ) + def test_deprecated_deadline(self): + with pytest.warns( + DeprecationWarning, match="This function should have been removed" + ): - @deprecated(deadline=(2000, 1, 1)) - def func_old(): - pass - - monkeypatch.undo() - - # No warn case 2: not in CI env - with warnings.catch_warnings(): - with patch("subprocess.run") as mock_run: - monkeypatch.delenv("CI", raising=False) - - # Mock "GITHUB_REPOSITORY" - monkeypatch.setenv("GITHUB_REPOSITORY", "TESTOWNER/TESTREPO") - mock_run.return_value.stdout.decode.return_value = ( - "git@github.com:TESTOWNER/TESTREPO.git" - ) - - @deprecated(deadline=(2000, 1, 1)) - def func_old_1(): - pass + @deprecated(deadline=(2000, 1, 1)) + def func_old(): + pass - monkeypatch.undo() + def test_deprecated_deadline_no_warn(self): + """No warning should be raised before deadline.""" - # No warn case 3: not in code owner repo with warnings.catch_warnings(): - monkeypatch.setenv("CI", "true") - monkeypatch.delenv("GITHUB_REPOSITORY", raising=False) + warnings.filterwarnings( + "error", "function should have been removed", DeprecationWarning + ) - @deprecated(deadline=(2000, 1, 1)) - def func_old_2(): + @deprecated(deadline=(9999, 1, 1)) + def func_old(): pass def test_requires(self): From 1a82e0be392589f1ed8635d3c3bcbf104b6699ea Mon Sep 17 00:00:00 2001 From: "Haoyu (Daniel)" Date: Fri, 3 Jan 2025 20:20:48 +0800 Subject: [PATCH 02/12] comment clean up --- src/monty/dev.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/monty/dev.py b/src/monty/dev.py index 320a022f7..e20512ab4 100644 --- a/src/monty/dev.py +++ b/src/monty/dev.py @@ -43,7 +43,7 @@ def deprecated( the choice accordingly. Returns: - Original function, but with a warning to use the updated function. + Original function/class, but with a warning to use the replacement. """ def craft_message( @@ -109,7 +109,7 @@ def new_init(self, *args, **kwargs): return cls - # Convert deadline to datetime type + # Convert deadline to `datetime` type _deadline: datetime | None = datetime(*deadline) if deadline is not None else None # Emit DeprecationWarning after removal deadline From ce1e5cb1183933c750c09d7af4bdea67d343ba0f Mon Sep 17 00:00:00 2001 From: "Haoyu (Daniel)" Date: Fri, 3 Jan 2025 20:37:39 +0800 Subject: [PATCH 03/12] Revert "drop repo check and warn instead of raise" This reverts commit 65931ce526615eb22199165d04c52ab3be3a9ea9. --- src/monty/dev.py | 54 ++++++++++++++++++++++++++++------ tests/test_dev.py | 74 ++++++++++++++++++++++++++++++++++++++--------- 2 files changed, 105 insertions(+), 23 deletions(-) diff --git a/src/monty/dev.py b/src/monty/dev.py index e20512ab4..8519270c7 100644 --- a/src/monty/dev.py +++ b/src/monty/dev.py @@ -8,6 +8,8 @@ import functools import inspect import logging +import os +import subprocess import sys import warnings from dataclasses import is_dataclass @@ -33,8 +35,8 @@ def deprecated( replacement (Callable | str): A replacement class or function. message (str): A warning message to be displayed. deadline (Optional[tuple[int, int, int]]): Optional deadline for removal - of the old function/class, in format (yyyy, MM, dd). DeprecationWarning - would be emitted after this date. + of the old function/class, in format (yyyy, MM, dd). A CI warning would + be raised after this date if is running in code owner' repo. category (Warning): Choose the category of the warning to issue. Defaults to FutureWarning. Another choice can be DeprecationWarning. Note that FutureWarning is meant for end users and is always shown unless silenced. @@ -46,6 +48,45 @@ def deprecated( Original function/class, but with a warning to use the replacement. """ + def raise_deadline_warning() -> None: + """Raise CI warning after removal deadline in code owner's repo.""" + + def _is_in_owner_repo() -> bool: + """Check if is running in code owner's repo. + Only generate reliable check when `git` is installed and remote name + is "origin". + """ + + try: + # Get current running repo + result = subprocess.run( + ["git", "config", "--get", "remote.origin.url"], + stdout=subprocess.PIPE, + ) + owner_repo = ( + result.stdout.decode("utf-8") + .strip() + .lstrip("https://github.com/") # HTTPS clone + .lstrip("git@github.com:") # SSH clone + .rstrip(".git") # SSH clone + ) + + return owner_repo == os.getenv("GITHUB_REPOSITORY") + + except (subprocess.CalledProcessError, FileNotFoundError): + return False + + # Only raise warning in code owner's repo CI + if ( + _deadline is not None + and os.getenv("CI") is not None + and datetime.now() > _deadline + and _is_in_owner_repo() + ): + raise DeprecationWarning( + f"This function should have been removed on {_deadline:%Y-%m-%d}." + ) + def craft_message( old: Callable, replacement: Callable | str, @@ -112,13 +153,8 @@ def new_init(self, *args, **kwargs): # Convert deadline to `datetime` type _deadline: datetime | None = datetime(*deadline) if deadline is not None else None - # Emit DeprecationWarning after removal deadline - if _deadline is not None and datetime.now() > _deadline: - warnings.warn( - f"This function should have been removed on {_deadline:%Y-%m-%d}.", - DeprecationWarning, - stacklevel=2, - ) + # Raise CI warning after removal deadline + raise_deadline_warning() def decorator(target: Callable) -> Callable: if inspect.isfunction(target): diff --git a/tests/test_dev.py b/tests/test_dev.py index 3451dd6dc..13bb211be 100644 --- a/tests/test_dev.py +++ b/tests/test_dev.py @@ -169,25 +169,71 @@ def method_b(self): assert old_class.__doc__ == "A dummy old class for tests." assert old_class.class_attrib_old == "OLD_ATTRIB" - def test_deprecated_deadline(self): - with pytest.warns( - DeprecationWarning, match="This function should have been removed" - ): + def test_deprecated_deadline(self, monkeypatch): + with pytest.raises(DeprecationWarning): + with patch("subprocess.run") as mock_run: + monkeypatch.setenv("CI", "true") # mock CI env + + # Mock "GITHUB_REPOSITORY" + monkeypatch.setenv("GITHUB_REPOSITORY", "TESTOWNER/TESTREPO") + mock_run.return_value.stdout.decode.return_value = ( + "git@github.com:TESTOWNER/TESTREPO.git" + ) + + @deprecated(deadline=(2000, 1, 1)) + def func_old(): + pass + + @pytest.fixture() + def test_deprecated_deadline_no_warn(self, monkeypatch): + """Test cases where no warning should be raised.""" + + # No warn case 1: date before deadline + with warnings.catch_warnings(): + with patch("subprocess.run") as mock_run: + monkeypatch.setenv("CI", "true") # mock CI env - @deprecated(deadline=(2000, 1, 1)) - def func_old(): - pass + # Mock date to 1999-01-01 + monkeypatch.setattr( + datetime.datetime, "now", datetime.datetime(1999, 1, 1) + ) - def test_deprecated_deadline_no_warn(self): - """No warning should be raised before deadline.""" + # Mock "GITHUB_REPOSITORY" + monkeypatch.setenv("GITHUB_REPOSITORY", "TESTOWNER/TESTREPO") + mock_run.return_value.stdout.decode.return_value = ( + "git@github.com:TESTOWNER/TESTREPO.git" + ) + @deprecated(deadline=(2000, 1, 1)) + def func_old(): + pass + + monkeypatch.undo() + + # No warn case 2: not in CI env with warnings.catch_warnings(): - warnings.filterwarnings( - "error", "function should have been removed", DeprecationWarning - ) + with patch("subprocess.run") as mock_run: + monkeypatch.delenv("CI", raising=False) + + # Mock "GITHUB_REPOSITORY" + monkeypatch.setenv("GITHUB_REPOSITORY", "TESTOWNER/TESTREPO") + mock_run.return_value.stdout.decode.return_value = ( + "git@github.com:TESTOWNER/TESTREPO.git" + ) - @deprecated(deadline=(9999, 1, 1)) - def func_old(): + @deprecated(deadline=(2000, 1, 1)) + def func_old_1(): + pass + + monkeypatch.undo() + + # No warn case 3: not in code owner repo + with warnings.catch_warnings(): + monkeypatch.setenv("CI", "true") + monkeypatch.delenv("GITHUB_REPOSITORY", raising=False) + + @deprecated(deadline=(2000, 1, 1)) + def func_old_2(): pass def test_requires(self): From 7f294d22ba0dee4d350f76dc3d299b1f742adab0 Mon Sep 17 00:00:00 2001 From: "Haoyu (Daniel)" Date: Fri, 3 Jan 2025 20:37:52 +0800 Subject: [PATCH 04/12] check upstream instead of origin --- src/monty/dev.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/monty/dev.py b/src/monty/dev.py index 8519270c7..990e5dcbc 100644 --- a/src/monty/dev.py +++ b/src/monty/dev.py @@ -54,13 +54,13 @@ def raise_deadline_warning() -> None: def _is_in_owner_repo() -> bool: """Check if is running in code owner's repo. Only generate reliable check when `git` is installed and remote name - is "origin". + is "upstream". """ try: # Get current running repo result = subprocess.run( - ["git", "config", "--get", "remote.origin.url"], + ["git", "config", "--get", "remote.upstream.url"], stdout=subprocess.PIPE, ) owner_repo = ( From 6488668a2ade564eff9e6d3bafe6dad053f6d79a Mon Sep 17 00:00:00 2001 From: "Haoyu (Daniel)" Date: Fri, 3 Jan 2025 20:39:26 +0800 Subject: [PATCH 05/12] use a weaker warn instead of raise --- src/monty/dev.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/monty/dev.py b/src/monty/dev.py index 990e5dcbc..0e5769d0b 100644 --- a/src/monty/dev.py +++ b/src/monty/dev.py @@ -83,8 +83,10 @@ def _is_in_owner_repo() -> bool: and datetime.now() > _deadline and _is_in_owner_repo() ): - raise DeprecationWarning( - f"This function should have been removed on {_deadline:%Y-%m-%d}." + warnings.warn( + f"This function should have been removed on {_deadline:%Y-%m-%d}.", + DeprecationWarning, + stacklevel=2, ) def craft_message( From 450fb2d1a681a36cd703da3cd19952744c42989c Mon Sep 17 00:00:00 2001 From: "Haoyu (Daniel)" Date: Fri, 3 Jan 2025 20:48:52 +0800 Subject: [PATCH 06/12] fix tests --- tests/test_dev.py | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/tests/test_dev.py b/tests/test_dev.py index 13bb211be..d0d936eb5 100644 --- a/tests/test_dev.py +++ b/tests/test_dev.py @@ -170,7 +170,9 @@ def method_b(self): assert old_class.class_attrib_old == "OLD_ATTRIB" def test_deprecated_deadline(self, monkeypatch): - with pytest.raises(DeprecationWarning): + with pytest.warns( + DeprecationWarning, match="This function should have been removed" + ): with patch("subprocess.run") as mock_run: monkeypatch.setenv("CI", "true") # mock CI env @@ -184,7 +186,6 @@ def test_deprecated_deadline(self, monkeypatch): def func_old(): pass - @pytest.fixture() def test_deprecated_deadline_no_warn(self, monkeypatch): """Test cases where no warning should be raised.""" @@ -193,18 +194,13 @@ def test_deprecated_deadline_no_warn(self, monkeypatch): with patch("subprocess.run") as mock_run: monkeypatch.setenv("CI", "true") # mock CI env - # Mock date to 1999-01-01 - monkeypatch.setattr( - datetime.datetime, "now", datetime.datetime(1999, 1, 1) - ) - # Mock "GITHUB_REPOSITORY" monkeypatch.setenv("GITHUB_REPOSITORY", "TESTOWNER/TESTREPO") mock_run.return_value.stdout.decode.return_value = ( "git@github.com:TESTOWNER/TESTREPO.git" ) - @deprecated(deadline=(2000, 1, 1)) + @deprecated(deadline=(9999, 1, 1)) def func_old(): pass From 19ed737e80511c5f6a4f35a71aa7c09253ba774d Mon Sep 17 00:00:00 2001 From: "Haoyu (Daniel)" Date: Fri, 3 Jan 2025 20:50:40 +0800 Subject: [PATCH 07/12] raise -> emit wording fix for warning --- src/monty/dev.py | 16 ++++++++-------- tests/test_dev.py | 2 +- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/monty/dev.py b/src/monty/dev.py index 0e5769d0b..68007a45a 100644 --- a/src/monty/dev.py +++ b/src/monty/dev.py @@ -35,8 +35,8 @@ def deprecated( replacement (Callable | str): A replacement class or function. message (str): A warning message to be displayed. deadline (Optional[tuple[int, int, int]]): Optional deadline for removal - of the old function/class, in format (yyyy, MM, dd). A CI warning would - be raised after this date if is running in code owner' repo. + of the old function/class, in format (yyyy, MM, dd). DeprecationWarning + would be emitted after this date if is running in code owner' repo. category (Warning): Choose the category of the warning to issue. Defaults to FutureWarning. Another choice can be DeprecationWarning. Note that FutureWarning is meant for end users and is always shown unless silenced. @@ -48,8 +48,8 @@ def deprecated( Original function/class, but with a warning to use the replacement. """ - def raise_deadline_warning() -> None: - """Raise CI warning after removal deadline in code owner's repo.""" + def emit_deadline_warning() -> None: + """Emit DeprecationWarning after removal deadline in code owner's repo.""" def _is_in_owner_repo() -> bool: """Check if is running in code owner's repo. @@ -58,7 +58,7 @@ def _is_in_owner_repo() -> bool: """ try: - # Get current running repo + # Get upstream (code owner) repo result = subprocess.run( ["git", "config", "--get", "remote.upstream.url"], stdout=subprocess.PIPE, @@ -76,7 +76,7 @@ def _is_in_owner_repo() -> bool: except (subprocess.CalledProcessError, FileNotFoundError): return False - # Only raise warning in code owner's repo CI + # Only emit warning in code owner's repo CI if ( _deadline is not None and os.getenv("CI") is not None @@ -155,8 +155,8 @@ def new_init(self, *args, **kwargs): # Convert deadline to `datetime` type _deadline: datetime | None = datetime(*deadline) if deadline is not None else None - # Raise CI warning after removal deadline - raise_deadline_warning() + # Emit CI warning after removal deadline + emit_deadline_warning() def decorator(target: Callable) -> Callable: if inspect.isfunction(target): diff --git a/tests/test_dev.py b/tests/test_dev.py index d0d936eb5..459ea83c7 100644 --- a/tests/test_dev.py +++ b/tests/test_dev.py @@ -187,7 +187,7 @@ def func_old(): pass def test_deprecated_deadline_no_warn(self, monkeypatch): - """Test cases where no warning should be raised.""" + """Test cases where no warning should be emitted.""" # No warn case 1: date before deadline with warnings.catch_warnings(): From 82049bd32c175dff75e3569d35d3481c53ce32b6 Mon Sep 17 00:00:00 2001 From: "Haoyu (Daniel)" Date: Fri, 3 Jan 2025 21:15:08 +0800 Subject: [PATCH 08/12] rearrange tests --- tests/test_dev.py | 67 ++++++++++++++++++++++++----------------------- 1 file changed, 34 insertions(+), 33 deletions(-) diff --git a/tests/test_dev.py b/tests/test_dev.py index 459ea83c7..3aeba4a61 100644 --- a/tests/test_dev.py +++ b/tests/test_dev.py @@ -1,6 +1,5 @@ from __future__ import annotations -import datetime import unittest import warnings from dataclasses import dataclass @@ -14,8 +13,8 @@ warnings.simplefilter("always") -class TestDecorator: - def test_deprecated(self): +class TestDeprecated: + def test_basic_usage(self): def func_replace(): pass @@ -35,7 +34,7 @@ def func_old(): assert func_old.__name__ == "func_old" assert func_old.__doc__ == "This is the old function." - def test_deprecated_str_replacement(self): + def test_str_replacement(self): @deprecated("func_replace") def func_old(): pass @@ -47,7 +46,7 @@ def func_old(): assert issubclass(w[0].category, FutureWarning) assert "use func_replace instead" in str(w[0].message) - def test_deprecated_property(self): + def test_property(self): class TestClass: """A dummy class for tests.""" @@ -76,7 +75,7 @@ def func_a(self): # Verify some things assert issubclass(w[-1].category, FutureWarning) - def test_deprecated_classmethod(self): + def test_classmethod(self): class TestClass: """A dummy class for tests.""" @@ -110,7 +109,7 @@ def classmethod_b(cls): with pytest.warns(DeprecationWarning): assert TestClass_deprecationwarning().classmethod_b() == "b" - def test_deprecated_class(self): + def test_class(self): class TestClassNew: """A dummy class for tests.""" @@ -137,7 +136,7 @@ def method_b(self): assert old_class.method_b.__doc__ == "This is method_b." - def test_deprecated_dataclass(self): + def test_dataclass(self): @dataclass class TestClassNew: """A dummy class for tests.""" @@ -169,7 +168,7 @@ def method_b(self): assert old_class.__doc__ == "A dummy old class for tests." assert old_class.class_attrib_old == "OLD_ATTRIB" - def test_deprecated_deadline(self, monkeypatch): + def test_deadline(self, monkeypatch): with pytest.warns( DeprecationWarning, match="This function should have been removed" ): @@ -186,7 +185,7 @@ def test_deprecated_deadline(self, monkeypatch): def func_old(): pass - def test_deprecated_deadline_no_warn(self, monkeypatch): + def test_deadline_no_warn(self, monkeypatch): """Test cases where no warning should be emitted.""" # No warn case 1: date before deadline @@ -232,34 +231,36 @@ def func_old_1(): def func_old_2(): pass - def test_requires(self): - try: - import fictitious_mod - except ImportError: - fictitious_mod = None - err_msg = "fictitious_mod is not present." +def test_requires(): + try: + import fictitious_mod + except ImportError: + fictitious_mod = None - @requires(fictitious_mod is not None, err_msg) - def use_fictitious_mod(): - print("success") + err_msg = "fictitious_mod is not present." - with pytest.raises(RuntimeError, match=err_msg): - use_fictitious_mod() + @requires(fictitious_mod is not None, err_msg) + def use_fictitious_mod(): + print("success") - @requires(unittest is not None, "unittest is not present.") - def use_unittest(): - return "success" + with pytest.raises(RuntimeError, match=err_msg): + use_fictitious_mod() - assert use_unittest() == "success" + @requires(unittest is not None, "unittest is not present.") + def use_unittest(): + return "success" - # test with custom error class - @requires(False, "expect ImportError", err_cls=ImportError) - def use_import_error(): - return "success" + assert use_unittest() == "success" - with pytest.raises(ImportError, match="expect ImportError"): - use_import_error() + # test with custom error class + @requires(False, "expect ImportError", err_cls=ImportError) + def use_import_error(): + return "success" - def test_install_except_hook(self): - install_excepthook() + with pytest.raises(ImportError, match="expect ImportError"): + use_import_error() + + +def test_install_except_hook(): + install_excepthook() From 279b265f75095541d03ee57d79b608ed77a01838 Mon Sep 17 00:00:00 2001 From: "Haoyu (Daniel)" Date: Fri, 3 Jan 2025 21:24:08 +0800 Subject: [PATCH 09/12] test https clone, avoid nested context manager --- tests/test_dev.py | 99 +++++++++++++++++++++++++++-------------------- 1 file changed, 58 insertions(+), 41 deletions(-) diff --git a/tests/test_dev.py b/tests/test_dev.py index 3aeba4a61..b6b460005 100644 --- a/tests/test_dev.py +++ b/tests/test_dev.py @@ -168,64 +168,81 @@ def method_b(self): assert old_class.__doc__ == "A dummy old class for tests." assert old_class.class_attrib_old == "OLD_ATTRIB" - def test_deadline(self, monkeypatch): - with pytest.warns( - DeprecationWarning, match="This function should have been removed" + @pytest.mark.parametrize( + "repo_url", + ( + "git@github.com:TESTOWNER/TESTREPO.git", # SSH clone + "https://github.com/TESTOWNER/TESTREPO.git", # HTTPS clone + ), + ) + def test_deadline(self, monkeypatch, repo_url): + with ( + pytest.warns( + DeprecationWarning, match="This function should have been removed" + ), + patch("subprocess.run") as mock_run, ): - with patch("subprocess.run") as mock_run: - monkeypatch.setenv("CI", "true") # mock CI env + monkeypatch.setenv("CI", "true") # mock CI env - # Mock "GITHUB_REPOSITORY" - monkeypatch.setenv("GITHUB_REPOSITORY", "TESTOWNER/TESTREPO") - mock_run.return_value.stdout.decode.return_value = ( - "git@github.com:TESTOWNER/TESTREPO.git" - ) + # Mock "GITHUB_REPOSITORY" to return "upstream URL" + monkeypatch.setenv("GITHUB_REPOSITORY", "TESTOWNER/TESTREPO") + mock_run.return_value.stdout.decode.return_value = repo_url - @deprecated(deadline=(2000, 1, 1)) - def func_old(): - pass + @deprecated(deadline=(2000, 1, 1)) + def func_old(): + pass def test_deadline_no_warn(self, monkeypatch): """Test cases where no warning should be emitted.""" - # No warn case 1: date before deadline - with warnings.catch_warnings(): - with patch("subprocess.run") as mock_run: - monkeypatch.setenv("CI", "true") # mock CI env + # Case 1: date before deadline + with warnings.catch_warnings(), patch("subprocess.run") as mock_run: + warnings.filterwarnings( + "error", "should have been removed", DeprecationWarning + ) - # Mock "GITHUB_REPOSITORY" - monkeypatch.setenv("GITHUB_REPOSITORY", "TESTOWNER/TESTREPO") - mock_run.return_value.stdout.decode.return_value = ( - "git@github.com:TESTOWNER/TESTREPO.git" - ) + monkeypatch.setenv("CI", "true") # mock CI env - @deprecated(deadline=(9999, 1, 1)) - def func_old(): - pass + # Mock "GITHUB_REPOSITORY" + monkeypatch.setenv("GITHUB_REPOSITORY", "TESTOWNER/TESTREPO") + mock_run.return_value.stdout.decode.return_value = ( + "git@github.com:TESTOWNER/TESTREPO.git" + ) - monkeypatch.undo() + @deprecated(deadline=(9999, 1, 1)) + def func_old(): + pass - # No warn case 2: not in CI env - with warnings.catch_warnings(): - with patch("subprocess.run") as mock_run: - monkeypatch.delenv("CI", raising=False) + monkeypatch.undo() - # Mock "GITHUB_REPOSITORY" - monkeypatch.setenv("GITHUB_REPOSITORY", "TESTOWNER/TESTREPO") - mock_run.return_value.stdout.decode.return_value = ( - "git@github.com:TESTOWNER/TESTREPO.git" - ) + # Case 2: not in CI env + with warnings.catch_warnings(), patch("subprocess.run") as mock_run: + warnings.filterwarnings( + "error", "should have been removed", DeprecationWarning + ) - @deprecated(deadline=(2000, 1, 1)) - def func_old_1(): - pass + monkeypatch.delenv("CI", raising=False) - monkeypatch.undo() + # Mock "GITHUB_REPOSITORY" + monkeypatch.setenv("GITHUB_REPOSITORY", "TESTOWNER/TESTREPO") + mock_run.return_value.stdout.decode.return_value = ( + "git@github.com:TESTOWNER/TESTREPO.git" + ) - # No warn case 3: not in code owner repo + @deprecated(deadline=(2000, 1, 1)) + def func_old_1(): + pass + + monkeypatch.undo() + + # Case 3: not in code owner (upstream) repo with warnings.catch_warnings(): + warnings.filterwarnings( + "error", "should have been removed", DeprecationWarning + ) + monkeypatch.setenv("CI", "true") - monkeypatch.delenv("GITHUB_REPOSITORY", raising=False) + monkeypatch.setenv("GITHUB_REPOSITORY", "OTHERUSER/OTHERREPO") @deprecated(deadline=(2000, 1, 1)) def func_old_2(): From b119f5ae8f1aaa1d9fee5375461f56302140de4d Mon Sep 17 00:00:00 2001 From: "Haoyu (Daniel)" Date: Fri, 3 Jan 2025 22:23:39 +0800 Subject: [PATCH 10/12] remove warning for deadline --- src/monty/dev.py | 49 +---------------------------- tests/test_dev.py | 80 ----------------------------------------------- 2 files changed, 1 insertion(+), 128 deletions(-) diff --git a/src/monty/dev.py b/src/monty/dev.py index 68007a45a..abe349fd8 100644 --- a/src/monty/dev.py +++ b/src/monty/dev.py @@ -8,8 +8,6 @@ import functools import inspect import logging -import os -import subprocess import sys import warnings from dataclasses import is_dataclass @@ -35,8 +33,7 @@ def deprecated( replacement (Callable | str): A replacement class or function. message (str): A warning message to be displayed. deadline (Optional[tuple[int, int, int]]): Optional deadline for removal - of the old function/class, in format (yyyy, MM, dd). DeprecationWarning - would be emitted after this date if is running in code owner' repo. + of the old function/class, in format (yyyy, MM, dd). category (Warning): Choose the category of the warning to issue. Defaults to FutureWarning. Another choice can be DeprecationWarning. Note that FutureWarning is meant for end users and is always shown unless silenced. @@ -48,47 +45,6 @@ def deprecated( Original function/class, but with a warning to use the replacement. """ - def emit_deadline_warning() -> None: - """Emit DeprecationWarning after removal deadline in code owner's repo.""" - - def _is_in_owner_repo() -> bool: - """Check if is running in code owner's repo. - Only generate reliable check when `git` is installed and remote name - is "upstream". - """ - - try: - # Get upstream (code owner) repo - result = subprocess.run( - ["git", "config", "--get", "remote.upstream.url"], - stdout=subprocess.PIPE, - ) - owner_repo = ( - result.stdout.decode("utf-8") - .strip() - .lstrip("https://github.com/") # HTTPS clone - .lstrip("git@github.com:") # SSH clone - .rstrip(".git") # SSH clone - ) - - return owner_repo == os.getenv("GITHUB_REPOSITORY") - - except (subprocess.CalledProcessError, FileNotFoundError): - return False - - # Only emit warning in code owner's repo CI - if ( - _deadline is not None - and os.getenv("CI") is not None - and datetime.now() > _deadline - and _is_in_owner_repo() - ): - warnings.warn( - f"This function should have been removed on {_deadline:%Y-%m-%d}.", - DeprecationWarning, - stacklevel=2, - ) - def craft_message( old: Callable, replacement: Callable | str, @@ -155,9 +111,6 @@ def new_init(self, *args, **kwargs): # Convert deadline to `datetime` type _deadline: datetime | None = datetime(*deadline) if deadline is not None else None - # Emit CI warning after removal deadline - emit_deadline_warning() - def decorator(target: Callable) -> Callable: if inspect.isfunction(target): return deprecated_function_decorator(target) diff --git a/tests/test_dev.py b/tests/test_dev.py index b6b460005..b7a0bacf8 100644 --- a/tests/test_dev.py +++ b/tests/test_dev.py @@ -168,86 +168,6 @@ def method_b(self): assert old_class.__doc__ == "A dummy old class for tests." assert old_class.class_attrib_old == "OLD_ATTRIB" - @pytest.mark.parametrize( - "repo_url", - ( - "git@github.com:TESTOWNER/TESTREPO.git", # SSH clone - "https://github.com/TESTOWNER/TESTREPO.git", # HTTPS clone - ), - ) - def test_deadline(self, monkeypatch, repo_url): - with ( - pytest.warns( - DeprecationWarning, match="This function should have been removed" - ), - patch("subprocess.run") as mock_run, - ): - monkeypatch.setenv("CI", "true") # mock CI env - - # Mock "GITHUB_REPOSITORY" to return "upstream URL" - monkeypatch.setenv("GITHUB_REPOSITORY", "TESTOWNER/TESTREPO") - mock_run.return_value.stdout.decode.return_value = repo_url - - @deprecated(deadline=(2000, 1, 1)) - def func_old(): - pass - - def test_deadline_no_warn(self, monkeypatch): - """Test cases where no warning should be emitted.""" - - # Case 1: date before deadline - with warnings.catch_warnings(), patch("subprocess.run") as mock_run: - warnings.filterwarnings( - "error", "should have been removed", DeprecationWarning - ) - - monkeypatch.setenv("CI", "true") # mock CI env - - # Mock "GITHUB_REPOSITORY" - monkeypatch.setenv("GITHUB_REPOSITORY", "TESTOWNER/TESTREPO") - mock_run.return_value.stdout.decode.return_value = ( - "git@github.com:TESTOWNER/TESTREPO.git" - ) - - @deprecated(deadline=(9999, 1, 1)) - def func_old(): - pass - - monkeypatch.undo() - - # Case 2: not in CI env - with warnings.catch_warnings(), patch("subprocess.run") as mock_run: - warnings.filterwarnings( - "error", "should have been removed", DeprecationWarning - ) - - monkeypatch.delenv("CI", raising=False) - - # Mock "GITHUB_REPOSITORY" - monkeypatch.setenv("GITHUB_REPOSITORY", "TESTOWNER/TESTREPO") - mock_run.return_value.stdout.decode.return_value = ( - "git@github.com:TESTOWNER/TESTREPO.git" - ) - - @deprecated(deadline=(2000, 1, 1)) - def func_old_1(): - pass - - monkeypatch.undo() - - # Case 3: not in code owner (upstream) repo - with warnings.catch_warnings(): - warnings.filterwarnings( - "error", "should have been removed", DeprecationWarning - ) - - monkeypatch.setenv("CI", "true") - monkeypatch.setenv("GITHUB_REPOSITORY", "OTHERUSER/OTHERREPO") - - @deprecated(deadline=(2000, 1, 1)) - def func_old_2(): - pass - def test_requires(): try: From 66d6b51529de2aa1778a226236a70f41f265a726 Mon Sep 17 00:00:00 2001 From: "Haoyu (Daniel)" Date: Fri, 3 Jan 2025 22:24:54 +0800 Subject: [PATCH 11/12] check deadline in warning msg --- tests/test_dev.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/test_dev.py b/tests/test_dev.py index b7a0bacf8..d8b33cdef 100644 --- a/tests/test_dev.py +++ b/tests/test_dev.py @@ -18,7 +18,7 @@ def test_basic_usage(self): def func_replace(): pass - @deprecated(func_replace, "Use func_replace instead") + @deprecated(func_replace, "Use func_replace instead", deadline=(2025, 1, 1)) def func_old(): """This is the old function.""" pass @@ -26,9 +26,10 @@ def func_old(): with warnings.catch_warnings(record=True) as w: # Trigger a warning. func_old() - # Verify Warning and message + assert issubclass(w[0].category, FutureWarning) assert "Use func_replace instead" in str(w[0].message) + assert "will be removed on 2025-01-01" in str(w[0].message) # Check metadata preservation assert func_old.__name__ == "func_old" From 740933bdf497a686e8229a655a8e24584c3566d0 Mon Sep 17 00:00:00 2001 From: "Haoyu (Daniel)" Date: Fri, 3 Jan 2025 22:30:41 +0800 Subject: [PATCH 12/12] drop unused logger --- src/monty/dev.py | 3 --- src/monty/logging.py | 2 -- 2 files changed, 5 deletions(-) diff --git a/src/monty/dev.py b/src/monty/dev.py index abe349fd8..e26808602 100644 --- a/src/monty/dev.py +++ b/src/monty/dev.py @@ -7,7 +7,6 @@ import functools import inspect -import logging import sys import warnings from dataclasses import is_dataclass @@ -17,8 +16,6 @@ if TYPE_CHECKING: from typing import Callable, Optional, Type -logger = logging.getLogger(__name__) - def deprecated( replacement: Optional[Callable | str] = None, diff --git a/src/monty/logging.py b/src/monty/logging.py index fed1b70de..03a4b813c 100644 --- a/src/monty/logging.py +++ b/src/monty/logging.py @@ -13,8 +13,6 @@ if TYPE_CHECKING: from typing import Callable -logger = logging.getLogger(__name__) - def logged(level: int = logging.DEBUG) -> Callable: """