-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
docs: Add deprecation policy to contributor guide #2900
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
Conversation
tpike3
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
|
LGTM - thank you for adding this. |
|
I'm a little confused, like in #2893 I've changed the code and examples but the migration guide and tutorials remain as is, following this document I should also update the docs and migration guide but then won't the users be unable to see the API before it was changed and won't be able to follow the tutorials. I was thinking of updating the tutorials and migration guide once the change will be changed to |
|
Great question! The prerequisites don’t all need to happen at once, they can be done anywhere between adding the alternative and the final deprecation. Notice the difference between may and must in the policy. The key is that everything is in place before/when we add the So your approach in #2893 is fine: add the alternative with a Regarding visibility of the old API: these changes are to our |
|
Just one clarifying question: why use |
|
@quaquel This is actually an interesting question. I looked into this and the distinction comes down to intended audience:
For Mesa, our users actively write code against our API: they subclass So it depends a bit if you see them as end-users or developers. They're probably both. Libraries with similar dynamics (NumPy, pandas, scikit-learn) use While I would like to check this though, because I do remember having seen But I think we should indeed update to |
|
pandas has a whole spec around this: PDEP-17. They start with We could adopt a similar pattern: |
This is something I have been struggling with myself as well. I see them more as end-users, but technically they are developers since they use our library in their own code.
If you run your own custom mesa model from within main, as any sane person should do, you'll get your deprecation warnings wherever we put them. What matters is just that you call mesa stuff that raises a deprecation warning directly or indirectly from a main.
I agree that this is the best approach going forward. |
Restructures the deprecation policy around a clear 4-step process: (1) introduce alternative with optional PendingDeprecationWarning (2) complete prerequisites (docs, migration guide, examples) (3) add FutureWarning for active deprecation (4) remove in next major version. Changes the active deprecation warning from DeprecationWarning to FutureWarning, which is visible to users by default. This is important since Mesa's deprecated methods live in imported modules, not user scripts, so DeprecationWarning would be hidden even when users run code in __main__.
|
I validated the behavior above, it's correctly described. I also thought about it a bit more, and I rewrote the guide to reflect that, and also made the order a bit more clear (it's now 4 logical steps). Please rereview. |
|
Merging, future additions/changes can be made later. |
Summary
Adds a new "Deprecation Policy" section to our contributor guide, establishing clear guidelines for how we deprecate features in Mesa.
Motive
As discussed in #1909 and various PR reviews (e.g., #2841 and #2872), we've been making ad-hoc decisions about deprecation practices. This has led to repeated discussions about which warning type to use, when deprecations can be added, and what prerequisites are needed. Having a documented policy will streamline future PRs and ensure consistent user experience.\
Please review critically: