Skip to content

Commit

Permalink
🩹Fix check_deprecations not showing deprecation warnings (#775)
Browse files Browse the repository at this point in the history
* ✨ Added 'deprecate_dict_entry' to deprecate dict keys and/or values
* 🧹📚 Changed  Warns OverDueDeprecation to Raises and added missing Raises OverDueDeprecation to deprecate_module_attribute and deprecate_submodule
* 🩹🧪 Reimplemented check_deprecations and with deprecate_dict_entry and renamed it to 'model_spec_deprecations'
This change also ensures that the deprecation warning is thrown when users calls 'load_model' so python will show the warning
* ♻️🧪 Changed deprecation_warning_on_call_test_helper to test for file emitting the warning and return the WarningsRecorder
* 📚 Added docs for deprecate_dict_entry
* 🧹 Removed unused record variables
* 🧪 Added test for deprecated 'spectral_constraints'
* 👌 Changed DeprecationWarning to GlotaranApiDeprecationWarning (GlotaranApiDeprecationWarning is a subclass of UserWarning so it won't be filtered automatically by python)
* 📚 Updated docs to reflect the usage of GlotaranApiDeprecationWarning
  • Loading branch information
s-weigand authored and jsnel committed Aug 14, 2021
1 parent b89196e commit 3f663cf
Show file tree
Hide file tree
Showing 14 changed files with 546 additions and 117 deletions.
16 changes: 15 additions & 1 deletion CONTRIBUTING.rst
Original file line number Diff line number Diff line change
Expand Up @@ -175,12 +175,15 @@ To make deprecations as robust as possible and give users all needed information
to adjust their code, we provide helper functions inside the module
:mod:`glotaran.deprecation`.

.. currentmodule:: glotaran.deprecation.deprecation_utils

The functions you most likely want to use are

* :func:`deprecate` for functions, methods and classes
* :func:`warn_deprecated` for call arguments
* :func:`deprecate_module_attribute` for module attributes
* :func:`deprecate_submodule` for modules
* :func:`deprecate_dict_entry` for dict entries


Those functions not only make it easier to deprecate something, but they also check that
Expand All @@ -193,7 +196,7 @@ provides the test helper functions ``deprecation_warning_on_call_test_helper`` a
Since the tests for deprecation are mainly for maintainability and not to test the
functionality (those tests should be in the appropriate place)
``deprecation_warning_on_call_test_helper`` will by default just test that a
``DeprecationWarning`` was raised and ignore all raise ``Exception`` s.
``GlotaranApiDeprecationWarning`` was raised and ignore all raise ``Exception`` s.
An exception to this rule is when adding back removed functionality
(which shouldn't happen in the first place but might), which should be
implemented in a file under ``glotaran/deprecation/modules`` and filenames should be like the
Expand Down Expand Up @@ -300,6 +303,17 @@ as an attribute to the parent package.
to_be_removed_in_version="0.6.0",
)
Deprecating dict entries
~~~~~~~~~~~~~~~~~~~~~~~~
The possible dict deprecation actions are:

- Swapping of keys ``{"foo": 1} -> {"bar": 1}`` (done via ``swap_keys=("foo", "bar")``)
- Replacing of matching values ``{"foo": 1} -> {"foo": 2}`` (done via ``replace_rules=({"foo": 1}, {"foo": 2})``)
- Replacing of matching values and swapping of keys ``{"foo": 1} -> {"bar": 2}`` (done via ``replace_rules=({"foo": 1}, {"bar": 2})``)

For full examples have a look at the examples from the docstring (:func:`deprecate_dict_entry`).


Deploying
---------

Expand Down
4 changes: 2 additions & 2 deletions glotaran/builtin/io/yml/yml.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

import yaml

from glotaran.deprecation.modules.builtin_io_yml import model_spec_deprecations
from glotaran.io import ProjectIoInterface
from glotaran.io import load_dataset
from glotaran.io import load_model
Expand All @@ -19,7 +20,6 @@
from glotaran.parameter import ParameterGroup
from glotaran.project import SavingOptions
from glotaran.project import Scheme
from glotaran.utils.sanitize import check_deprecations
from glotaran.utils.sanitize import sanitize_yaml

if TYPE_CHECKING:
Expand Down Expand Up @@ -49,7 +49,7 @@ def load_model(self, file_name: str) -> Model:
with open(file_name) as f:
spec = yaml.safe_load(f)

check_deprecations(spec)
model_spec_deprecations(spec)

spec = sanitize_yaml(spec)

Expand Down
1 change: 1 addition & 0 deletions glotaran/deprecation/__init__.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
"""Deprecation helpers and place to put deprecated implementations till removing."""
from glotaran.deprecation.deprecation_utils import deprecate
from glotaran.deprecation.deprecation_utils import deprecate_dict_entry
from glotaran.deprecation.deprecation_utils import deprecate_module_attribute
from glotaran.deprecation.deprecation_utils import deprecate_submodule
from glotaran.deprecation.deprecation_utils import warn_deprecated
172 changes: 167 additions & 5 deletions glotaran/deprecation/deprecation_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,15 @@
from typing import TYPE_CHECKING
from typing import Any
from typing import Callable
from typing import Hashable
from typing import Mapping
from typing import MutableMapping
from typing import TypeVar
from typing import cast
from warnings import warn

import numpy as np

DecoratedCallable = TypeVar(
"DecoratedCallable", bound=Callable[..., Any]
) # decorated function or class
Expand All @@ -32,6 +37,20 @@ class OverDueDeprecation(Exception):
warn_deprecated
deprecate_module_attribute
deprecate_submodule
deprecate_dict_entry
"""


class GlotaranApiDeprecationWarning(UserWarning):
"""Warning to give users about API changes.
See Also
--------
deprecate
warn_deprecated
deprecate_module_attribute
deprecate_submodule
deprecate_dict_entry
"""


Expand Down Expand Up @@ -166,8 +185,8 @@ def warn_deprecated(
will import ``package.module.class`` and check if ``class`` has an attribute
``mapping``.
Warns
-----
Raises
------
OverDueDeprecation
If the current version is greater or equal to ``end_of_life_version``.
Expand Down Expand Up @@ -212,7 +231,7 @@ def read_parameters_from_yaml_file(model_path: str):
selected_indices = importable_indices[: len(selected_qual_names)]
check_qualnames_in_tests(qual_names=selected_qual_names, importable_indices=selected_indices)
warn(
DeprecationWarning(
GlotaranApiDeprecationWarning(
f"Usage of {deprecated_qual_name_usage!r} was deprecated, "
f"use {new_qual_name_usage!r} instead.\n"
f"This usage will be an error in version: {to_be_removed_in_version!r}."
Expand Down Expand Up @@ -265,8 +284,8 @@ def deprecate(
DecoratedCallable
Original function or class throwing a Deprecation warning when used.
Warns
-----
Raises
------
OverDueDeprecation
If the current version is greater or equal to ``end_of_life_version``.
Expand Down Expand Up @@ -334,6 +353,134 @@ def outer_wrapper(deprecated_object: DecoratedCallable) -> DecoratedCallable:
return cast(Callable[[DecoratedCallable], DecoratedCallable], outer_wrapper)


def deprecate_dict_entry(
*,
dict_to_check: MutableMapping[Hashable, Any],
deprecated_usage: str,
new_usage: str,
to_be_removed_in_version: str,
swap_keys: tuple[Hashable, Hashable] | None = None,
replace_rules: tuple[Mapping[Hashable, Any], Mapping[Hashable, Any]] | None = None,
stacklevel: int = 3,
) -> None:
"""Replace dict entry inplace and warn about usage change, if present in the dict.
Parameters
----------
dict_to_check : MutableMapping[Hashable, Any]
Dict which should be checked.
deprecated_usage : str
Old usage to inform user (only used in warning).
new_usage : str
New usage to inform user (only used in warning).
to_be_removed_in_version : str
Version the support for this usage will be removed.
swap_keys : tuple[Hashable, Hashable]
(old_key, new_key),
``dict_to_check[new_key]`` will be assigned the value ``dict_to_check[old_key]``
and ``old_key`` will be removed from the dict.
by default None
replace_rules : Mapping[Hashable, tuple[Any, Any]]
({old_key: old_value}, {new_key: new_value}),
If ``dict_to_check[old_key]`` has the value ``old_value``,
``dict_to_check[new_key]`` it will be set to ``new_value``.
``old_key`` will be removed from the dict if ``old_key`` and ``new_key`` aren't equal.
by default None
stacklevel : int
Stack at which the warning should be shown as raise. , by default 3
Raises
------
ValueError
If both ``swap_keys`` and ``replace_rules`` are None (default) or not None.
OverDueDeprecation
If the current version is greater or equal to ``end_of_life_version``.
See Also
--------
warn_deprecated
Notes
-----
To prevent confusion exactly one of ``replace_rules`` and ``swap_keys``
needs to be passed.
Examples
--------
For readability sake the warnings won't be shown in the examples.
Swapping key names:
>>> dict_to_check = {"foo": 123}
>>> deprecate_dict_entry(
dict_to_check=dict_to_check,
deprecated_usage="foo",
new_usage="bar",
to_be_removed_in_version="0.6.0",
swap_keys=("foo", "bar")
)
>>> dict_to_check
{"bar": 123}
Changing values:
>>> dict_to_check = {"foo": 123}
>>> deprecate_dict_entry(
dict_to_check=dict_to_check,
deprecated_usage="foo: 123",
new_usage="foo: 123.0",
to_be_removed_in_version="0.6.0",
replace_rules=({"foo": 123}, {"foo": 123.0})
)
>>> dict_to_check
{"foo": 123.0}
Swapping key names AND changing values:
>>> dict_to_check = {"type": "kinetic-spectrum"}
>>> deprecate_dict_entry(
dict_to_check=dict_to_check,
deprecated_usage="type: kinectic-spectrum",
new_usage="default-megacomplex: decay",
to_be_removed_in_version="0.6.0",
replace_rules=({"type": "kinetic-spectrum"}, {"default-megacomplex": "decay"})
)
>>> dict_to_check
{"default-megacomplex": "decay"}
.. # noqa: DAR402
"""
dict_changed = False

if not np.logical_xor(swap_keys is None, replace_rules is None):
raise ValueError(
"Exactly one of the parameters `swap_keys` or `replace_rules` needs to be provided."
)
if swap_keys is not None and swap_keys[0] in dict_to_check:
dict_changed = True
dict_to_check[swap_keys[1]] = dict_to_check[swap_keys[0]]
del dict_to_check[swap_keys[0]]
if replace_rules is not None:
old_key, old_value = next(iter(replace_rules[0].items()))
new_key, new_value = next(iter(replace_rules[1].items()))
if old_key in dict_to_check and dict_to_check[old_key] == old_value:
dict_changed = True
dict_to_check[new_key] = new_value
if new_key != old_key:
del dict_to_check[old_key]

if dict_changed:
warn_deprecated(
deprecated_qual_name_usage=deprecated_usage,
new_qual_name_usage=new_usage,
to_be_removed_in_version=to_be_removed_in_version,
stacklevel=stacklevel,
check_qual_names=(False, False),
)


def module_attribute(module_qual_name: str, attribute_name: str) -> Any:
"""Import and return the attribute (e.g. function or class) of a module.
Expand Down Expand Up @@ -384,6 +531,11 @@ def deprecate_module_attribute(
Any
Module attribute from its new location.
Raises
------
OverDueDeprecation
If the current version is greater or equal to ``end_of_life_version``.
See Also
--------
deprecate
Expand Down Expand Up @@ -412,6 +564,8 @@ def __getattr__(attribute_name: str):
raise AttributeError(f"module {__name__} has no attribute {attribute_name}")
.. # noqa: DAR402
"""
module_name = ".".join(new_qual_name.split(".")[:-1])
attribute_name = new_qual_name.split(".")[-1]
Expand Down Expand Up @@ -457,6 +611,11 @@ def deprecate_submodule(
ModuleType
Module containing
Raises
------
OverDueDeprecation
If the current version is greater or equal to ``end_of_life_version``.
See Also
--------
deprecate
Expand All @@ -478,6 +637,9 @@ def deprecate_submodule(
new_module_name="glotaran.project.result",
to_be_removed_in_version="0.6.0",
)
.. # noqa: DAR402
"""
new_module = import_module(new_module_name)
deprecated_module = ModuleType(
Expand Down
Loading

0 comments on commit 3f663cf

Please sign in to comment.