Repository: OceanParcels/Parcels
Pull request #1683 (2024-09-03 15:26:22)

Add deprecation decorators
Adds deprecation decorators `@deprecated()` and `@deprecated_made_private`. Note the brackets on `deprecated()` when using it. This is because it accepts an optional custom message that is appended to the warning message (to be used to denote an alternative to users, or any other information that may be of use).


Note: When using in conjunction with other decorators (e.g., `@staticmethod` `@property`) the deprecation method should be the innermost one (i.e., closest to the `def ...`) so that they're applied first.

Examples _(updated)_:
```python
from parcels.tools._helpers import deprecated, deprecated_made_private

class SomeClass:
    @deprecated("Please use some_method_other_method instead.") # deprecated in v3.1 # TODO: Remove in future version
    def some_method(self, x, y):
        return x + y
    
    @deprecated_made_private # deprecated in v3.1 # TODO: Remove in future version
    def private_method(self):
        pass


x = SomeClass().some_method(1, 2)
SomeClass().private_method()

print(x)
```
output:
```
<filename>:<line_num>: DeprecationWarning: `SomeClass.some_method` is deprecated and will be removed in a future release of Parcels. Please use some_method_other_method instead.
  x = SomeClass().some_method(1, 2)
<filename>:<line_num>: DeprecationWarning: `SomeClass.private_method` is deprecated and will be removed in a future release of Parcels. It was identified as part of the internal API as it is not expected to be directly used by the end-user. If you feel that you use this code directly in your scripts, please comment on our tracking issue at <>.
  SomeClass().private_method()

3
```

codecov[bot]: ## [Codecov](https://app.codecov.io/gh/OceanParcels/Parcels/pull/1683?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=OceanParcels) Report
Attention: Patch coverage is `31.57895%` with `13 lines` in your changes missing coverage. Please review.

| [Files with missing lines](https://app.codecov.io/gh/OceanParcels/Parcels/pull/1683?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=OceanParcels) | Patch % | Lines |
|---|---|---|
| [parcels/util/\_helpers.py](https://app.codecov.io/gh/OceanParcels/Parcels/pull/1683?src=pr&el=tree&filepath=parcels%2Futil%2F_helpers.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=OceanParcels#diff-cGFyY2Vscy91dGlsL19oZWxwZXJzLnB5) | 31.57% | [13 Missing :warning: ](https://app.codecov.io/gh/OceanParcels/Parcels/pull/1683?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=OceanParcels) |

> :exclamation:  There is a different number of reports uploaded between BASE (93cc635) and HEAD (d8dab5a). Click for more details.
> 
> <details><summary>HEAD has 8 uploads less than BASE</summary>
>
>| Flag | BASE (93cc635) | HEAD (d8dab5a) |
>|------|------|------|
>|unit-tests|5|0|
>|integration-tests|3|0|
></details>

<details><summary>Additional details and impacted files</summary>


```diff
@@             Coverage Diff             @@
##           master    #1683       +/-   ##
===========================================
- Coverage   81.10%   15.59%   -65.52%     
===========================================
  Files          70       43       -27     
  Lines       13197     7870     -5327     
  Branches      127      127               
===========================================
- Hits        10704     1227     -9477     
- Misses       2456     6516     +4060     
- Partials       37      127       +90     
```

| [Flag](https://app.codecov.io/gh/OceanParcels/Parcels/pull/1683/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=OceanParcels) | Coverage Δ | |
|---|---|---|
| [integration-tests](https://app.codecov.io/gh/OceanParcels/Parcels/pull/1683/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=OceanParcels) | `?` | |
| [mypy](https://app.codecov.io/gh/OceanParcels/Parcels/pull/1683/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=OceanParcels) | `15.59% <31.57%> (+0.03%)` | :arrow_up: |
| [unit-tests](https://app.codecov.io/gh/OceanParcels/Parcels/pull/1683/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=OceanParcels) | `?` | |

Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=OceanParcels#carryforward-flags-in-the-pull-request-comment) to find out more.

| [Files with missing lines](https://app.codecov.io/gh/OceanParcels/Parcels/pull/1683?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=OceanParcels) | Coverage Δ | |
|---|---|---|
| [parcels/util/\_helpers.py](https://app.codecov.io/gh/OceanParcels/Parcels/pull/1683?src=pr&el=tree&filepath=parcels%2Futil%2F_helpers.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=OceanParcels#diff-cGFyY2Vscy91dGlsL19oZWxwZXJzLnB5) | `31.57% <31.57%> (ø)` | |

... and [64 files with indirect coverage changes](https://app.codecov.io/gh/OceanParcels/Parcels/pull/1683/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=OceanParcels)

</details>
VeckoTheGecko: The creation of the `util` directory is a bit unjustified in this PR. Perhaps it would be better to put somewhere else. I wasn't too fussed about organisation since I anticipate needing to create more private only helper functions/modules and didn't mind kicking that can down the road. Open to ideas :')
erikvansebille: > The creation of the `util` directory is a bit unjustified in this PR. Perhaps it would be better to put somewhere else. I wasn't too fussed about organisation since I anticipate needing to create more private only helper functions/modules and didn't mind kicking that can down the road. Open to ideas :')

I'm fine with creating this subdirectory; we can move other tests in there too at a later stage
VeckoTheGecko: My motivation for creating the `parcels/util` directory was more "ah, we'll probably need some more internal tooling down the line". On second thought - rather than anticipating a need that might not arise - I think it would be better to just put it in `tools`. Since its internal tooling we can move it at a later stage without expecting disruptions.

> we can move other tests in there too at a later stage

I didn't quite understand this comment. Do we need to reconsider the folder structure of the tests?
erikvansebille: > I didn't quite understand this comment. Do we need to reconsider the folder structure of the tests?

Maybe it was a bit vague. I meant to say we can reconsider where to put which test files at a later stage. Like what you suggest above, so all fine

Patch:
diff --git a/parcels/tools/_helpers.py b/parcels/tools/_helpers.py
new file mode 100644
index 00000000..ffe58315
--- /dev/null
+++ b/parcels/tools/_helpers.py
@@ -0,0 +1,53 @@
+"""Internal helpers for Parcels."""
+
+import functools
+import warnings
+from typing import Callable
+
+PACKAGE = "Parcels"
+
+
+def deprecated(msg: str = "") -> Callable:
+    """Decorator marking a function as being deprecated
+
+    Parameters
+    ----------
+    msg : str, optional
+        Custom message to append to the deprecation warning.
+
+    Examples
+    --------
+    ```
+    @deprecated("Please use `another_function` instead")
+    def some_old_function(x, y):
+        return x + y
+
+    @deprecated()
+    def some_other_old_function(x, y):
+        return x + y
+    ```
+    """
+    if msg:
+        msg = " " + msg
+
+    def decorator(func):
+        @functools.wraps(func)
+        def wrapper(*args, **kwargs):
+            msg_formatted = (
+                f"`{func.__qualname__}` is deprecated and will be removed in a future release of {PACKAGE}.{msg}"
+            )
+
+            warnings.warn(msg_formatted, category=DeprecationWarning, stacklevel=2)
+            return func(*args, **kwargs)
+
+        return wrapper
+
+    return decorator
+
+
+def deprecated_made_private(func: Callable) -> Callable:
+    return deprecated(
+        "It has moved to the internal API as it is not expected to be directly used by "
+        "the end-user. If you feel that you use this code directly in your scripts, please "
+        "comment on our tracking issue at <>."  # TODO: Add tracking issue
+    )(func)


Test patch:
diff --git a/tests/tools/test_helpers.py b/tests/tools/test_helpers.py
new file mode 100644
index 00000000..0089fed3
--- /dev/null
+++ b/tests/tools/test_helpers.py
@@ -0,0 +1,53 @@
+import pytest
+
+from parcels.tools._helpers import deprecated, deprecated_made_private
+
+
+def test_deprecated():
+    class SomeClass:
+        @deprecated()
+        def some_method(self, x, y):
+            return x + y
+
+        @staticmethod
+        @deprecated()
+        def some_static_method(x, y):
+            return x + y
+
+        @property
+        @deprecated()
+        def some_property(self):
+            return 2
+
+    @deprecated()
+    def some_function(x, y):
+        return x + y
+
+    with pytest.warns(DeprecationWarning) as record:
+        SomeClass().some_method(1, 2)
+    assert "SomeClass.some_method" in record[0].message.args[0]
+
+    with pytest.warns(DeprecationWarning) as record:
+        SomeClass.some_static_method(1, 2)
+    assert "SomeClass.some_static_method" in record[0].message.args[0]
+
+    with pytest.warns(DeprecationWarning) as record:
+        _ = SomeClass().some_property
+    assert "SomeClass.some_property" in record[0].message.args[0]
+
+    with pytest.warns(DeprecationWarning) as record:
+        some_function(1, 2)
+    assert "some_function" in record[0].message.args[0]
+
+    with pytest.warns(DeprecationWarning) as record:
+        some_function(1, 2)
+    assert "some_function" in record[0].message.args[0]
+
+
+def test_deprecated_made_private():
+    @deprecated_made_private
+    def some_function(x, y):
+        return x + y
+
+    with pytest.warns(DeprecationWarning):
+        some_function(1, 2)