Skip to content

Commit

Permalink
Merge pull request #735 from DanielYang59/fix-deprecated-repo-check
Browse files Browse the repository at this point in the history 
Drop `deadline` warning in `dev.deprecated`
  • Loading branch information
@shyuep
shyuep authored Jan 3, 2025
2 parents 26acf0b + 740933b commit 8c13b0f
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 153 deletions.
56 changes: 4 additions & 52 deletions src/monty/dev.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,6 @@

import functools
import inspect
import logging
import os
import subprocess
import sys
import warnings
from dataclasses import is_dataclass
Expand All @@ -19,8 +16,6 @@
if TYPE_CHECKING:
from typing import Callable, Optional, Type

logger = logging.getLogger(__name__)


def deprecated(
replacement: Optional[Callable | str] = None,
Expand All @@ -35,8 +30,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). 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).
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.
Expand All @@ -45,48 +39,9 @@ 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 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("[email protected]:") # 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,
Expand Down Expand Up @@ -150,11 +105,8 @@ 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()
# Convert deadline to `datetime` type
_deadline: datetime | None = datetime(*deadline) if deadline is not None else None

def decorator(target: Callable) -> Callable:
if inspect.isfunction(target):
Expand Down
2 changes: 0 additions & 2 deletions src/monty/logging.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,6 @@
if TYPE_CHECKING:
from typing import Callable

logger = logging.getLogger(__name__)


def logged(level: int = logging.DEBUG) -> Callable:
"""
Expand Down
133 changes: 34 additions & 99 deletions tests/test_dev.py
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
from __future__ import annotations

import datetime
import unittest
import warnings
from dataclasses import dataclass
Expand All @@ -14,28 +13,29 @@
warnings.simplefilter("always")


class TestDecorator:
def test_deprecated(self):
class TestDeprecated:
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

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"
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
Expand All @@ -47,7 +47,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."""

Expand Down Expand Up @@ -76,7 +76,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."""

Expand Down Expand Up @@ -110,7 +110,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."""

Expand All @@ -137,7 +137,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."""
Expand Down Expand Up @@ -169,101 +169,36 @@ 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 = (
"[email protected]: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 = (
"[email protected]: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():
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 = (
"[email protected]:TESTOWNER/TESTREPO.git"
)

@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):
try:
import fictitious_mod
except ImportError:
fictitious_mod = None
def test_requires():
try:
import fictitious_mod
except ImportError:
fictitious_mod = None

err_msg = "fictitious_mod is not present."

err_msg = "fictitious_mod is not present."
@requires(fictitious_mod is not None, err_msg)
def use_fictitious_mod():
print("success")

@requires(fictitious_mod is not None, err_msg)
def use_fictitious_mod():
print("success")
with pytest.raises(RuntimeError, match=err_msg):
use_fictitious_mod()

with pytest.raises(RuntimeError, match=err_msg):
use_fictitious_mod()
@requires(unittest is not None, "unittest is not present.")
def use_unittest():
return "success"

@requires(unittest is not None, "unittest is not present.")
def use_unittest():
return "success"
assert use_unittest() == "success"

assert use_unittest() == "success"
# test with custom error class
@requires(False, "expect ImportError", err_cls=ImportError)
def use_import_error():
return "success"

# test with custom error class
@requires(False, "expect ImportError", err_cls=ImportError)
def use_import_error():
return "success"
with pytest.raises(ImportError, match="expect ImportError"):
use_import_error()

with pytest.raises(ImportError, match="expect ImportError"):
use_import_error()

def test_install_except_hook(self):
install_excepthook()
def test_install_except_hook():
install_excepthook()

0 comments on commit 8c13b0f

Please sign in to comment.