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 #3174

Merged
merged 8 commits into from Jul 19, 2023

Conversation

CSSFrancis
Copy link
Member

@CSSFrancis CSSFrancis commented Jun 19, 2023

Description of the change

This adds 2 helper function into hyperspy to improve the deprecation handling and have it more inline with downstream packages (kikchipy/ pyxem).

A decorator can be added to a function to track if it is deprecated or if an argument is deprecated with a potential replacement argument. If there is a replacement argument that value will be passed in as the replacement argument and (for better or worse) the function will not fail.

I found that this streamlines the deprecation process and helps to improve consistency.

Progress of the PR

  • Change implemented (can be split into several points),
  • update docstring (if appropriate),
  • update user guide (if appropriate),
  • add an changelog entry in the upcoming_changes folder (see upcoming_changes/README.rst),
  • add tests,
  • ready for review.

Minimal example of the bug fix or the new feature

@deprecated(since=2.0, alternative="new_function", removal=2.1)
def deprecated_function():
    pass

@deprecated_argument(name="a", since="2.0", removal="2.1", alternative="b")
def function(self, b):
    return b

This is mostly copied from @hakonanes and kikchipy with some slight changes

@CSSFrancis CSSFrancis changed the base branch from RELEASE_next_minor to RELEASE_next_major June 19, 2023 20:31
@codecov
Copy link

codecov bot commented Jun 19, 2023

Codecov Report

Patch coverage: 96.07% and project coverage change: +0.02 🎉

Comparison is base (8ef1a73) 81.31% compared to head (994915b) 81.33%.

Additional details and impacted files
@@                  Coverage Diff                   @@
##           RELEASE_next_major    #3174      +/-   ##
======================================================
+ Coverage               81.31%   81.33%   +0.02%     
======================================================
  Files                     176      176              
  Lines                   24406    24457      +51     
  Branches                 5681     5688       +7     
======================================================
+ Hits                    19845    19892      +47     
- Misses                   3257     3259       +2     
- Partials                 1304     1306       +2     
Impacted Files Coverage Δ
hyperspy/decorators.py 90.82% <96.07%> (+4.61%) ⬆️

... and 3 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@ericpre ericpre changed the title Add deprecation Add deprecation decorators Jul 2, 2023
@ericpre
Copy link
Member

ericpre commented Jul 2, 2023

@CSSFrancis, is the intended use internal only or is it something that would be expected to be used by other libraries? If the later, I am wondering, if this is something that would be suitable for https://scientific-python.org? If the former, it would only need documentation in the developer guide.

@CSSFrancis
Copy link
Member Author

I think internal use only. My opinion is that we want to be able to change and adjust it as we go.

It might potentially be good to have a standard in the entire scipy ecosystem for deprecating code but everyone copying code isn't a bad thing as it does give a little more freedom.

I'll update the developer guide.

Copy link
Contributor

@hakonanes hakonanes left a comment

Choose a reason for hiding this comment

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

This looks clean to me. I've pointed out a few typos and some style inconsistencies.

doc/dev_guide/coding_style.rst Outdated Show resolved Hide resolved
doc/dev_guide/coding_style.rst Outdated Show resolved Hide resolved
This will update the docstring, and print a visible deprecation warning telling the user to user the
alternative function or argument.

These deprecation wrappers are copied from work by Håkon Wiik Ånes and kikchipy.
Copy link
Contributor

Choose a reason for hiding this comment

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

While this note is nice, I suggest to rephrase to something like "The deprecation wrappers are inspired by those in kikuchipy". Or, I'm OK with also just removing it: so much of kikuchipy is inspired by how things are handled in HyperSpy, without exhaustive references back to HyperSpy (although there are a few), so I'm just happy that some functionality "flows" the other way.

Copy link
Member Author

Choose a reason for hiding this comment

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

Sounds good :) I'll just leave that they came from kikuchipy.

I do think they are quite a good way to handle deprecations and it might be good to promote them for wider usage. If someone takes them from hyperspy it would be good to have the credit make it's way back to you!

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks, that is more than enough (:

hyperspy/decorators.py Outdated Show resolved Hide resolved
hyperspy/decorators.py Show resolved Hide resolved
hyperspy/decorators.py Show resolved Hide resolved
hyperspy/tests/test_decorators.py Outdated Show resolved Hide resolved
@CSSFrancis CSSFrancis merged commit 8de42e3 into hyperspy:RELEASE_next_major Jul 19, 2023
24 checks passed
@CSSFrancis CSSFrancis deleted the AddDeprecation branch July 19, 2023 14:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants