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

Add deprecation decorators #1683

Merged
merged 2 commits into from
Sep 4, 2024
Merged

Add deprecation decorators #1683

merged 2 commits into from
Sep 4, 2024

Conversation

VeckoTheGecko
Copy link
Contributor

@VeckoTheGecko VeckoTheGecko commented Sep 3, 2024

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):

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

Copy link

codecov bot commented Sep 3, 2024

Codecov Report

Attention: Patch coverage is 98.30508% with 1 line in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
parcels/tools/_helpers.py 95.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1683      +/-   ##
==========================================
+ Coverage   81.10%   81.18%   +0.07%     
==========================================
  Files          70       72       +2     
  Lines       13197    13256      +59     
  Branches      127      127              
==========================================
+ Hits        10704    10762      +58     
- Misses       2456     2457       +1     
  Partials       37       37              
Flag Coverage Δ
integration-tests 64.83% <ø> (ø)
mypy 15.65% <55.00%> (+0.10%) ⬆️
unit-tests 81.11% <100.00%> (+0.08%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
tests/tools/test_helpers.py 100.00% <100.00%> (ø)
parcels/tools/_helpers.py 95.00% <95.00%> (ø)

@VeckoTheGecko
Copy link
Contributor Author

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 :')

@VeckoTheGecko VeckoTheGecko force-pushed the v/deprecation-tooling branch 2 times, most recently from b86f524 to a533f4c Compare September 3, 2024 15:45
parcels/util/_helpers.py Outdated Show resolved Hide resolved
tests/util/test_helpers.py Outdated Show resolved Hide resolved
@erikvansebille
Copy link
Member

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
Copy link
Contributor Author

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
Copy link
Member

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

@VeckoTheGecko VeckoTheGecko merged commit 5fff42c into master Sep 4, 2024
13 checks passed
@VeckoTheGecko VeckoTheGecko deleted the v/deprecation-tooling branch September 4, 2024 14:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants