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
HEP 2: Release and deprecation policies #388
base: main
Are you sure you want to change the base?
Conversation
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.
Overall, this HEP is great.
However, I'm very much against being unable to put a version of when a deprecation is removed.
Some technical details have also made it into this HEP. An example of this is the release project section. It is not that it is wrong, but if we, at some point, want to change it to no longer be triggered by pushing a tag, we need to update this HEP and get the approval.
doc/heps/hep2.md
Outdated
|
||
When a feature has been deprecated, its warning and documentation must stay in place for some time long enough to let most users find out about it. For example, Panel users who lock the dependencies of their application should be given sufficient time between the deprecation of a feature and its removal so as not to miss the deprecation warnings and be left with broken code, once they attempt to upgrade their version of Panel to the latest. A deprecated feature **can only be removed 18 months after users have been informed of its deprecation**. This period, introduced by this HEP, factors in the facts that: | ||
|
||
- Users cannot build expectations about when in time a feature is going to be removed if the deprecation indicates a version number (e.g. `'This feature will be removed in version 1.2.3'`) as the Projects don't release new versions based on a regular cadence. |
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.
I don't buy this. Adding a version when the removal is being made is fine for both the users and us as developers.
I also think defining the deprecation period of 18 months is too long for some packages (Panel) because a lot of releases can have happened in that time period, whereas for other packages, it may not be enough (colorcet). In a worst-case scenario, a user could download a package on the last day of the 17th month and start using it, only to see it removed the next day because of a new release.
By keeping a table, updating it, and checking it, we also add extra burden on us as maintainers to keep it up to date. Having the deprecations as close to the code as possible makes it easier to avoid it from getting out of sync.
Again, I'm very much on having at least the option to put the version where the removal is gonna happen in the warning message, like what Matplotlib and xarray does. To be clear, this is not what all packages do. An example of this is pandas, which postpone it to the future, but they seem to be moving towards more major releases with breaking changes.
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.
Adding a version when the removal is being made is fine for both the users and us as developers.
My main goal here is to define a minimum deprecation period that all HoloViz projects can adhere to. I think it'd have benefits for its users and maintainers. I could see allowing projects to specify the removal version as long as they ensure that it meets the minimum deprecation period. For example, if a project releases 1.0.0
and announces a feature will be removed in 1.3.0
, the feature can only be removed X months after 1.0.0
, even if 1.3.0
happens before.
I also think defining the deprecation period of 18 months is too long for some packages (Panel) because a lot of releases can have happened in that period, whereas for other packages, it may not be enough (colorcet).
I don't think the number or frequency of releases matters so much for the deprecation period. The deprecation period is mostly a tradeoff between letting users have enough time to adapt (apps and libraries) and reducing the cost of maintaining a project. I want HoloViz as a whole project to agree on a minimum deprecation period. Individual projects are free to adopt longer deprecation periods.
In a worst-case scenario, a user could download a package on the last day of the 17th month and start using it, only to see it removed the next day because of a new release.
This isn't a very good counter-example, as this is all about probability, and in that case well, bad luck! The same could happen with a shorter or longer deprecation period. Just to be clear, I'm not saying you should make a new release right after the deprecation period is over.
By keeping a table, updating it, and checking it, we also add extra burden on us as maintainers to keep it up to date. Having the deprecations as close to the code as possible makes it easier to avoid it from getting out of sync.
I don't think HoloViz projects are used to deprecating and removing features, while I'd argue they should do it more to clean up their ever-growing API that sometimes confuses their users. For instance, I personally rarely check whether some code could be removed when I do a new minor release. I haven't built that muscle working on HoloViz projects. How about other maintainers? So I'm suggesting here a systematic approach to dealing with deprecated features, I don't think it's so much extra burden, and I anyway think it's part of the maintainer's job and more precisely of the release manager.
Again, I'm very much on having at least the option to put the version where the removal is gonna happen in the warning message, like what Matplotlib and xarray does. To be clear, this is not what all packages do. An example of this is pandas, which postpone it to the future, but they seem to be moving towards more major releases with breaking changes.
- Matplotlib has great docs on how to deprecate a feature (https://matplotlib.org/devdocs/devel/api_changes.html#rules), it's a minimum of 2 minor releases later and possibly more. Its 3.x releases were: 3.8 (09/2023), 3.7 (02/2023), 3.6 (06/2022), 3.5 (11/2021), 3.4 (03/2021), 3.3 (07/2020), 3.2 (03/2020), 3.1 (05/2019), 3.0 (09/2018). The time between two minor releases is in months: 18, 14, 12, 16, 15, 15 and 15. It's a pretty stable release cadence so I think it's fair for them to announce they will remove a feature in 2 releases. HoloViz projects could also decide to adopt a regular-ish release cadence, though it might be more extra burden than what I'm suggesting.
- Xarray's contributor docs doesn't say you should say when a feature is removed, maybe their docs and code isn't in sync https://docs.xarray.dev/en/stable/contributing.html#backwards-compatibility.
doc/heps/hep2.md
Outdated
|
||
## Release process | ||
|
||
Releasing a new version consists of packaging the software and distributing it. The Projects are set up to automatically perform this process on a Continuous Integration (CI) system (e.g. Github Actions). The process is triggered automatically when a new tag (of the form `v1.2.3`) is pushed to the Git repository. The tag message should be of the form `Version x.x.x` for final releases and `Version x.x.x. alpha1/beta1/RC1` for pre-releases. |
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.
I think the tag message should be removed or at least loosened, as it is, in my opinion, not important.
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.
I suggested that because I observed that the message was pretty inconsistent across releases in a project, and across projects. I like consistency. But you're also right that it is not the most important part of the HEP.
doc/heps/hep2.md
Outdated
|
||
- `alpha` (e.g. `1.2.3a1`): Projects can deliver alpha releases for users to benefit from and/or test bug fixes and new features, early in the development phase of a new version. alpha releases are common across the Projects. | ||
- `beta` (e.g. `1.2.3b1`): Projects can deliver beta releases for the same reasons, except they indicate the project is in a more advanced development phase of a new version. beta releases are not common across the Projects, they're more likely to be useful when a major release is in development, to progressively deliver large new features and API breaking changes. | ||
- `release candidate` (e.g. `1.2.3rc1`): Projects must deliver a release candidate version before the final release. |
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.
This could also explicitly say that no new features can be added after the first RC, and only regressions and bugs should be handled.
Must is also a strong word. I would say another of the dev releases is also good enough for a patch release.
An expectation of a release candidate for some is that it has a time period where people have a chance to test it. What I see in our cases is that we make a release candidate to test if our CI runs correctly (with downstream test), and when it is finished, we make the official release.
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.
This could also explicitly say that no new features can be added after the first RC, and only regressions and bugs should be handled.
Yes, I didn't think about that this is interesting.
Must is also a strong word. I would say another of the dev releases is also good enough for a patch release.
I'm suggesting replacing the alpha releases (often just one) made before a patch release with release candidates.
An expectation of a release candidate for some is that it has a time period where people have a chance to test it.
Yes, that's true. The HEP could explain that release candidates are always announced sometime before the final release.
What I see in our cases is that we make a release candidate to test if our CI runs correctly (with downstream test), and when it is finished, we make the official release.
Not always, we have applications that test release candidates before the final release. It's not the most common case though, even if it's difficult to say whether other users rely on dev releases in their internal code bases.
Thanks for your review @hoxbro !
I've commented on that in #388 (comment).
Yep true that's a bit too technical and I can remove it, it's already documented in the HoloViz Contributing Guide. But I have to say I'm not sure yet what exactly could and should get into a HEP. For instance, Matplotlib developer docs (https://matplotlib.org/devdocs/devel/) include a section with MEPs and many other sections on e.g. Dependency version policy (the equivalent of our HEP 1) or API Changes (the equivalent of this HEP). So in which case should we need a new HEP (if we need HEPs at all?)? @droumis pinging you since you told me you were thinking about writing HEP0. |
I just talked with Simon and told him I was open to adopting another way to define the deprecation period, as long as there's some sort of consistency across the projects. I suggested a minimum deprecation period of 18 months, he prefers a minimum number of minor versions (correct me if I'm wrong). I still prefer a minimum deprecation period, but I think that in the end whatever we adopt will be better than the status quo! |
Simon opened holoviz/geoviews#701 to remove some features of GeoViews. I started to have a look at it and felt it was a good use case, and well, it was just in front of my eyes at the right time! This helps me understand the whole deprecation process and will hopefully help improve the HEP. By the way, this is all very factual, I don't intend to blame anyone :) Shared
|
My take away from this as a maintainer:
|
Made some changes:
I think more work is needed:
|
I finally got around to dedicating some time to the HEP and updated it in 43cb239. I probably need to give it a good review with fresh eyes, but the next step for me will be to introduce it to the team in a HoloViz meeting. |
@philippjfr here are some deprecation periods for other libraries:
18 months is for sure quite high. Listening to your suggestions on:
|
@jlstevens suggested it would be great if there was automation to enforce some of the requirements. I think that:
|
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.
Looks good, added a couple suggestions
doc/heps/hep2.md
Outdated
The Projects already follow a pretty consistent procedure when it comes to releases, which this HEP formalizes. However, the Projects have applied various approaches for their deprecation cycle, and without clear guidelines this has sometimes led to long and unproductive discussions. The HEP aims to improve this by introducing new and well defined guidelines. Making it easier for Projects to deprecate features is **key to their long-term maintenance**. | ||
|
||
The overall goal of the HEP is to ensure that these policies are **consistently applied across the Projects**, which will help provide a **consistent user experience** and **help contributors and maintainers make decisions**. We also aim to adopt **standard practices**. | ||
|
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.
Let's add a concise 'Resolution' section after 'Summary' for a quick recap of the technical plan of action. Something like:
## Resolution
- Adopt a tiered warning strategy for deprecations:
- Initially emit a `DeprecationWarning`, ensuring it's set with the appropriate stacklevel for accurate source identification.
- After a 12-month period, elevate the warning to a `FutureWarning` to capture the attention of all users, including those who might not be configured to see DeprecationWarnings.
- Allow exceptions in rare cases, such as beginning with a `PendingDeprecationWarning` for potentially reversible or highly disruptive changes, or immediately using a `FutureWarning` for urgent cases.
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.
I don't really want to summarize the plan of action as I think all the steps of the deprecation cycle are important.
If automation is too high a bar, then the next best thing is establishing a clear, concrete process that does things correctly. For instance, maybe we should have a standardized approach/template for a 'deprecation PR' that we need to create every release. That way a maintainer can just look up at a reference deprecation PR (e.g. the previous one), follow the steps and without too much cognitive effort be fairly confident that the recommendations in this HEP have been followed. At a minimum I would want a standardized PR name (e.g. Deprecations for version X.X.X) and the first comment to link to this HEP! |
But isn't the clear process the |
Yes but you also want something concrete as well that integrates well into your release cycle. Checking the I'm trying to suggest an approach to make our lives easier, not harder! :-) |
doc/heps/hep2.md
Outdated
|
||
- `patch` releases (e.g. 0.1.**0** -> 0.1.**1**) should never intentionally and rarely, if ever, unintentionally break API. Should be safe to upgrade to the latest patch release if you encounter any problems. | ||
- `minor` releases (e.g. 0.**1**.1 -> 0.**2**.0) should not have API changes that affect most users. API changes in minor releases should be rare, but are not unheard of, particularly for recently added functionality whose API is still being refined, or for bugs found in older code that can't be fixed without changing API at least slightly. | ||
- `major` releases (e.g. **0**.2.0 -> **1**.0.0) will typically break APIs, but should normally include significant new functionality to motivate users to update their code. Major breaking API changes should be postponed to a major release and paired with significant user-visible advantages. |
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.
I would remove the sentence about "significant new functionality", I think we should be perfectly comfortable making a major release solely to make API changes.
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.
The sentence says should normally include significant new functionality
so it doesn't make it a strong requirement.
By the way, I literally copied these definitions from a comment from @jbednar holoviz/panel#5221 (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.
Right after that comment I wrote "I'd be interested in whether other team members think this description is accurate.", but no one engaged with it. Reading it now, I'm happy for @philippjfr to make the call -- would we normally provide some carrot along with the stick, i.e. provide some enticement to make it worth switching? You make more releases than I do, so I leave it to your discretion.
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.
FWIW I re-used your description as I found it appropriate.
Co-authored-by: Andrew <15331990+ahuang11@users.noreply.github.com>
|
HEP2 is ready for review. I think the first reviews and discussions have helped resolve some points of contention, but there may still be some. According to HEP0, we should aim to reach a consensus. To make sure I capture everyone's perspective, please add a comment. It could be as simple as 👍, if there are parts of the HEP you disagree with please raise them so they can be further discussed. cc @jbednar @philippjfr @jlstevens @MarcSkovMadsen @hoxbro @droumis @ahuang11 |
minor comment about docs, but either way 👍 |
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.
I've given it a detailed review, and made a bunch of tiny wording fixes and clarifications, none of which I think should be controversial. I have a couple of minor questions below, but overall, I'm happy with it and happy for it to be merged. Good job! (And I vote for a 6 month period; longer than that and we will have no memory of what we were doing!)
doc/heps/hep2.md
Outdated
@@ -54,7 +54,7 @@ Aside from certain exceptional cases, the Projects are not expected to backport | |||
HoloViz release managers are responsible for distributing the Projects on these platforms: | |||
|
|||
- Pre-releases are distributed on [PyPI](https://pypi.org) and on the *pyviz/label/dev* channel of [Anaconda.org](https://anaconda.org). | |||
- Final versions are distributed on [PyPI](https://pypi.org), and on the *conda-forge* and *pyviz* channels of [Anaconda.org](https://anaconda.org). | |||
- Final versions are distributed on [PyPI](https://pypi.org), and on the *conda-forge* and *pyviz* channels of [Anaconda.org](https://anaconda.org), and (in many cases) on the *main* channel as well. |
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.
@jbednar I didn't include the main channel originally as that feels like this shouldn't be the responsibility of a HoloViz maintainer, but of an Anaconda employee, and it turns out the intersection of both is close to 100% at the moment (it may not always be the case).
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.
Left some minor comments but I'm quite happy with this! Really appreciate the effort you put into this.
Co-authored-by: Philipp Rudiger <prudiger@anaconda.com>
Co-authored-by: Simon Høxbro Hansen <simon.hansen@me.com>
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.
Looks good to me!
Thanks for driving this forward @maximlt !
Thanks all for the reviews! With approvals from @droumis, @jbednar, @philippjfr, @hoxbro, @jlstevens, and @ahuang11 (one thumb up somewhere 🙃 ), I think it's fair to consider we've reached a consensus, HEP2 is approved! So next time you intend to deprecate a feature somewhere, please check the HEP :) (I'll now see with Demetris how to merge it since it depends on #386) |
I don't think merging this depends on exposing HEPs on the website. It certainly wasn't a blocker for HEP1. Let's go ahead and merge this and then continue the website work. |
This PR was branched off #386. Instead, I rebased it on
@droumis, could you have a final look and merge if it's all okay? |
Warning: builds on #386
This PR adds
HEP 2: Release and deprecation policies
. It is motivated by a discussion we had in holoviz/panel#5221, and by my interest in ensuring that the HoloViz projects apply consistent policies, both for its users and maintainers. The HEP formalizes practices that have been in place for a long time and adds a few more rules on top of those. For instance, I've tried to come up with a clearer process for maintainers to adopt when they deprecate a feature. I've also suggested a minimum deprecation period of 18 months before removal, I'm sure there will be opinions about this period, and I'm also quite convinced we need to define such a period.I drew some inspiration from:
I haven't touched all things on this topic, either because I didn't want to or didn't know what to suggest, I'd say these can be out of scope for now: