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

Use deprecation package for deprecation warnings #2028

Open
brosaplanella opened this issue Apr 20, 2022 · 4 comments
Open

Use deprecation package for deprecation warnings #2028

brosaplanella opened this issue Apr 20, 2022 · 4 comments
Labels
difficulty: easy A good issue for someone new. Can be done in a few hours priority: low No existing plans to resolve

Comments

@brosaplanella
Copy link
Member

Replace existing deprecation warnings/errors with deprecation package so tests fail after a few releases.

@brosaplanella
Copy link
Member Author

I have been looking into this and I have a (probably very basic) question. The deprecation package uses decorators and it seems designed to deprecate entire methods directly. How should I use them when we want to deprecate either a few lines of code, or some arguments? For an example, see l84 of simulation.py.

@Saransh-cpp
Copy link
Member

Saransh-cpp commented Jun 3, 2022

The deprecat package (built on top of the deprecate package) allows us to put deprecation warnings for arguments - https://github.com/mjhajharia/deprecat#deprecated-kwargs. Not sure how we can deprecate some particular lines of code.

(deprecat was built as deprecate doesn't allow deprecating arguments - https://deprecat.readthedocs.io/en/latest/source/introduction.html#some-background)

@agriyakhetarpal
Copy link
Member

@brosaplanella I was looking through some solutions for this and while I couldn't find anything to deprecate some lines of code, there are indeed ways for deprecating arguments: in addition to deprecat mentioned by @Saransh-cpp, there's also pyDeprecate – a similar library with many more customisations available, applicable on both classes and methods along with forwarding inputs to newer arguments.

Both of them will come at the cost of an extra dependency, so here is a purely Pythonic way which could perhaps be implemented as helper functions in pybamm/doc_utils.py? (see #2597). It transfers parameter values too but can be modified to just raise a warning on deprecation: https://gist.github.com/ZoomQuiet/f0113711181750e56ddfb51756d5c7fc

@Saransh-cpp
Copy link
Member

pyDeprecate looks really nice. We could also implement a new helper function, but having an extra dependency doesn't sound too bad to me (if it fulfills all our requirements).

@rtimms rtimms added difficulty: easy A good issue for someone new. Can be done in a few hours priority: low No existing plans to resolve labels May 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
difficulty: easy A good issue for someone new. Can be done in a few hours priority: low No existing plans to resolve
Projects
None yet
Development

No branches or pull requests

4 participants