accelerate
accelerate copied to clipboard
🚀 [Feature] Add deprecated Decorator
What does this PR do?
Summary
This PR introduces a deprecated decorator, designed to mark functions as deprecated with detailed warnings, following the approach in torch/onnx/_deprecation.py. The decorator provides a standard way to issue deprecation warnings and update docstrings, informing users of the version when a function was deprecated, its removal target, and recommended alternative actions.
Changes
- New
deprecateddecorator: Adds a decorator that marks functions as deprecated, integrates a warning when the function is called, and updates docstrings accordingly. - Integration with
FindTiedParametersResult: Applied thedeprecateddecorator to thevaluesmethod in theFindTiedParametersResultclass, signaling to users that this method will be removed in Accelerate v1.3.0 and suggesting alternative approaches.
Example Usage
After this PR, the following usage:
from accelerate.utils.modeling import FindTiedParametersResult
results = FindTiedParametersResult([[1, 2, 3]])
results.values()
will produce output similar to:
<stdin>:1: FutureWarning: 'accelerate.utils.modeling.values' is deprecated in version 1.0.0rc0 and will be removed in 1.3.0. Please use another method instead.
[2, 3]
Testing
Included a test for deprecated to validate:
- Warning output when a deprecated function is called.
- Docstring formatting to ensure it includes the deprecation note with the appropriate information.
Maybe not relevant, but Python added its own warnings.deprecated decorator in Python 3.13. Using the same name could be confusing. The Python decorator apparently also allows type checkers to flag the usage, which would be nice to have. Of course, we can't use it directly since we want to support older Python versions.
Maybe not relevant, but Python added its own
warnings.deprecateddecorator in Python 3.13. Using the same name could be confusing. The Python decorator apparently also allows type checkers to flag the usage, which would be nice to have. Of course, we can't use it directly since we want to support older Python versions.
Hello @BenjaminBossan, thank you for the insightful review! I wasn’t aware of the warnings.deprecated decorator introduced in Python 3.13, and I appreciate you pointing it out. As I am currently working with a lower Python version (likely 3.8, according to the Dev Container), I implemented a custom decorator to ensure compatibility.
That said, the name overlap could indeed be confusing. Would using an alternative name for the decorator help clarify things? I’d be glad to make adjustments if needed!
As to the Python version, 3.8 is going to be end of life by the end of the month, so the containers will be updated soon. Still, it will be quite some time before 3.13 will be the lowest supported Python version, so we can't assume that the decorator exists. More importantly, I wonder if we can "borrow" some of the implementation details of warnings.deprecated, as I'm sure the Python devs put a lot of thought into it. Regarding the naming, I'll let Zach decide on that.
Thank you for your feedback, I agree with your suggestions regarding naming and compatibility! Using Python’s warnings.deprecated decorator in 3.13 and above, while defaulting to our custom decorator in lower versions, sounds like a balanced approach. This way, we maintain compatibility across versions while taking advantage of the built-in decorator when possible.
I’ve resolved the conflicts in the code for now, and I’ll be sure to apply any further adjustments based on additional feedback. Thanks again!
The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.
Hi folks,
Could we consider using the implementation from typing_extensions>=4.5 instead? It could maybe be wrapped still to provide help with formatting for UX/docs, but that way we get improved IDE support and alignment with PEP 702.
@matthewdouglas AFAICT, that implementation is a backport of warnings.deprecated that we discussed above. I agree that typing support would be nice to have, but feature-wise, the two are not on par, so not sure if we can simply replace it.
I hope you're all doing well! I wanted to follow up on this PR to check if there are any remaining concerns or additional feedback that I should address to move it forward. Please let me know if there's anything else I can do to improve this PR :) Thank you for your time and guidance!
This issue has been automatically marked as stale because it has not had recent activity. If you think this still needs to be addressed please comment on this thread.
Please note that issues that do not follow the contributing guidelines are likely to be ignored.
not stale
This issue has been automatically marked as stale because it has not had recent activity. If you think this still needs to be addressed please comment on this thread.
Please note that issues that do not follow the contributing guidelines are likely to be ignored.