Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce @deprecated decorators for functions and enforce warnings as errors in tests #4757

Open
wants to merge 26 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 21 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
ff6a817
Deprecation for deprecated fucntions
RohitP2005 Jan 16, 2025
7f34a0b
style: pre-commit fixes
pre-commit-ci[bot] Jan 16, 2025
3da7140
Merge branch 'pybamm-team:develop' into issue#2028
RohitP2005 Feb 2, 2025
3a15fe8
Removed previous deprecation warnings
RohitP2005 Feb 2, 2025
9c500b5
Merge branch 'pybamm-team:develop' into issue#2028
RohitP2005 Feb 3, 2025
949571b
Added -W flag to pytest
RohitP2005 Feb 3, 2025
3f327cb
Custom decorator added and filterwarningn adjusted
RohitP2005 Feb 4, 2025
9d29350
Merge branch 'develop' into issue#2028
RohitP2005 Feb 5, 2025
1eb61a0
setuptools fix
RohitP2005 Feb 10, 2025
47051ab
Merge branch 'develop' into issue#2028
RohitP2005 Feb 10, 2025
e57a1fd
using agriyakhetarpal's approach to ignore external warnings
RohitP2005 Feb 13, 2025
dba1881
Merge branch 'develop' into issue#2028
RohitP2005 Feb 13, 2025
c48ed38
Merge branch 'pybamm-team:develop' into issue#2028
RohitP2005 Feb 14, 2025
b91e9ce
Update pyproject.toml
RohitP2005 Feb 15, 2025
c564c87
moving deprecation decorator to uitl.py
RohitP2005 Feb 14, 2025
52e610b
Merge branch 'develop' into issue#2028
RohitP2005 Feb 20, 2025
a4025a1
style: pre-commit fixes
pre-commit-ci[bot] Feb 20, 2025
33ff787
Moved decorators to utils and corrected pytest config
RohitP2005 Feb 20, 2025
9d9c1bf
Added msg parameter in deprecation decorator
RohitP2005 Feb 20, 2025
139704c
Applied decorators to necessary functions
RohitP2005 Feb 20, 2025
808d5cd
Corrected imports for utils
RohitP2005 Feb 20, 2025
28f5397
Update pyproject.toml
RohitP2005 Feb 20, 2025
a703eca
Update pyproject.toml
RohitP2005 Feb 20, 2025
8784b35
removed old deprecation warningsdfasf
RohitP2005 Feb 20, 2025
7902c43
deprecation Decorator accepts msg as a dictionary
RohitP2005 Feb 20, 2025
eaf0994
Merge branch 'develop' into issue#2028
arjxn-py Feb 22, 2025
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ dependencies = [
"posthog",
"pyyaml",
"platformdirs",
"deprecation"
]

[project.urls]
Expand Down Expand Up @@ -216,6 +217,8 @@ required_plugins = [
addopts = [
"-nauto",
"-vra",
"-ra",
"--showlocals",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm curious: is there a reason you think we should opt for this option? Do you feel that our tracebacks are limited?

"--strict-config",
"--strict-markers",
]
Expand All @@ -229,7 +232,9 @@ filterwarnings = [
# ignore internal nbmake warnings
'ignore:unclosed \<socket.socket:ResourceWarning',
'ignore:unclosed event loop \<:ResourceWarning',
# ignore warnings generated while running tests
# convert internal warnings as errors
"error::DeprecationWarning:pybamm.*",
# ignore external DeprecationWarning
"ignore::DeprecationWarning",
"ignore::UserWarning",
"ignore::RuntimeWarning",
Expand Down
13 changes: 13 additions & 0 deletions src/pybamm/expression_tree/symbol.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import pybamm
from pybamm.util import import_optional_dependency
from pybamm.expression_tree.printing.print_name import prettify_print_name
from deprecation import deprecated

if TYPE_CHECKING: # pragma: no cover
import casadi
Expand Down Expand Up @@ -356,6 +357,12 @@ def domain(self, domain):
)

@property
@deprecated(
deprecated_in="25.1.0",
removed_in="26.0.0",
current_version=pybamm.__version__,
details="Use `symbol.domains` instead.",
)
def auxiliary_domains(self):
"""Returns auxiliary domains."""
raise NotImplementedError(
Expand Down Expand Up @@ -994,6 +1001,12 @@ def create_copy(
children = self._children_for_copying(new_children)
return self.__class__(self.name, children, domains=self.domains)

@deprecated(
deprecated_in="25.1.0",
removed_in="26.0.0",
current_version=pybamm.__version__,
details="Use `create_copy` instead.",
)
def new_copy(
self,
new_children: list[Symbol] | None = None,
Expand Down
11 changes: 11 additions & 0 deletions src/pybamm/parameters/parameter_values.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
from pprint import pformat
from warnings import warn
from collections import defaultdict
from pybamm.util import deprecate_arguments


class ParameterValues:
Expand Down Expand Up @@ -417,6 +418,16 @@ def set_initial_ocps(
return parameter_values

@staticmethod
@deprecate_arguments(
deprecated_args={
"electrode diffusivity": "Use 'particle diffusivity' instead.",
"1 + dlnf/dlnc": "Use 'Thermodynamic factor' instead.",
},
deprecated_in="24.1.0",
removed_in="25.3.0",
current_version="25.1.0",
msg="Update your parameter names to avoid future compatibility issues.",
)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we have multiple messages per argument? The current message here is not informative enough, and is rather a step back in comparison to the messages that currently exist already. That is, it would be nice if @deprecate_arguments() could take a dictionary, i.e., accept a key-value pair for an argument and its corresponding deprecation message. The current approach you've taken doesn't seem to accommodate this behaviour.

def check_parameter_values(values):
for param in list(values.keys()):
if "propotional term" in param:
Expand Down
14 changes: 7 additions & 7 deletions src/pybamm/simulation.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
from datetime import timedelta
import pybamm.telemetry
from pybamm.util import import_optional_dependency

from deprecation import deprecated
from pybamm.expression_tree.operations.serialise import Serialise


Expand Down Expand Up @@ -156,8 +156,6 @@ def __setstate__(self, state):
self.get_esoh_solver = lru_cache()(self._get_esoh_solver)

def set_up_and_parameterise_experiment(self, solve_kwargs=None):
msg = "pybamm.simulation.set_up_and_parameterise_experiment is deprecated and not meant to be accessed by users."
warnings.warn(msg, DeprecationWarning, stacklevel=2)
self._set_up_and_parameterise_experiment(solve_kwargs=solve_kwargs)

def _set_up_and_parameterise_experiment(self, solve_kwargs=None):
Expand Down Expand Up @@ -238,11 +236,13 @@ def _set_up_and_parameterise_experiment(self, solve_kwargs=None):
parameterised_model
)

@deprecated(
deprecated_in="25.1.0",
removed_in="26.0.0",
current_version=pybamm.__version__,
details="pybamm.set_parameters is deprecated and not meant to be accessed by users.",
)
def set_parameters(self):
msg = (
"pybamm.set_parameters is deprecated and not meant to be accessed by users."
)
warnings.warn(msg, DeprecationWarning, stacklevel=2)
self._set_parameters()

def _set_parameters(self):
Expand Down
8 changes: 8 additions & 0 deletions src/pybamm/solvers/base_solver.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import pybamm
from pybamm.expression_tree.binary_operators import _Heaviside
from pybamm import ParameterValues
from pybamm.util import deprecate_arguments


class BaseSolver:
Expand Down Expand Up @@ -1162,6 +1163,13 @@ def process_t_interp(self, t_interp):

return t_interp

@deprecate_arguments(
deprecated_args={"npts": "Use 't_eval' instead."},
deprecated_in="25.1.0",
removed_in="25.3.0",
current_version="25.1.0",
msg="Please refer to the updated documentation.",
)
def step(
self,
old_solution,
Expand Down
45 changes: 45 additions & 0 deletions src/pybamm/util.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,9 @@
import pickle
import timeit
import difflib
import warnings
from warnings import warn
import functools

import pybamm

Expand Down Expand Up @@ -408,3 +410,46 @@ def import_optional_dependency(module_name, attribute=None):
except ModuleNotFoundError as error:
# Raise an ModuleNotFoundError if the module or attribute is not available
raise ModuleNotFoundError(err_msg) from error


def deprecate_arguments(
deprecated_args: dict[str, str],
deprecated_in: str,
removed_in: str,
current_version: str,
msg: str = "",
):
"""
Custom decorator to deprecate specific function arguments with an optional custom message.

Args:
deprecated_args (dict): Dictionary of deprecated argument names with details.
Example: {"old_arg": "Use 'new_arg' instead."}
deprecated_in (str): Version when the argument was deprecated.
removed_in (str): Version when the argument will be removed.
current_version (str): Current version of the package/module.
msg (str, optional): Additional custom message for the warning. Defaults to "".
"""

def decorator(func):
@functools.wraps(func)
def wrapper(*args, **kwargs):
for arg, message in deprecated_args.items():
if arg in kwargs:
warning_message = (
f"Argument '{arg}' is deprecated since version {deprecated_in} "
f"and will be removed in version {removed_in}. (Current version: {current_version}) "
f"{message}"
)
if msg:
warning_message += f" {msg}"
warnings.warn(
warning_message,
category=DeprecationWarning,
stacklevel=2,
)
return func(*args, **kwargs)

return wrapper

return decorator
Loading