-
-
Notifications
You must be signed in to change notification settings - Fork 25.9k
MAINT Use towncrier for changelog management #30046
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
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 like this overall, but one thing I'd change is that having so many folders mirroring our modules kinda looks / feels dirty. I'd rather have the file name specify that as <PR#>.<TYPE>.<SECTION>.rst
instead.
We should also fix our linter checking for changelog, and might also want a linter to check the file name itself.
🎉
I don't think this is possible with towncrier unfortunately, feel free to have a look at the doc in case I missed something ...
There is https://github.com/scientific-python/action-towncrier-changelog that does most of it I think. I have started testing it this morning in lesteve#37 and it looks good enough for our purposes. |
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. We're most probably gonna need to tune / fix a few things during the release process anyway.
Does this mean we can simplify the code in https://github.com/scikit-learn/scikit-learn/blob/main/.github/workflows/check-changelog.yml ? |
I don't know about simplifying but the CI part will need to be adapted. As mentioned in #30046 (comment) there is scientific-python/action-towncrier-changelog that we can use. I have started testing in lesteve#37 and it looks good enough for our purposes. I think we still have some custom logic that says if no tests were changed then a changelog is not required. If we want to keep this logic, this needs to be implemented I guess as a first step in the CI job. |
I want to point out that these categories that are listed in the changelog instructions:
are now playing a much bigger role than ever. Before, we simply put [ENH] or [MNT] in front of the change and the changes were not sortable by these categories. Now they are, if I understand correctly. But these are not really good categories, they should not overlap, they should cover everything. Major concerns:
|
What does this implement/fix? Explain your changes.
This is a step towards using towncrier for easier management of our changelog.
The config and template are in place, so feed-back is more than welcome. I have added a short README.rst that explains how to create a fragment file (single file with a naming convention like
<PR_NUMBER>.<TYPE>.rst
that you need to add in the right folder for each PR to describe the change).I added a few fragment files that I created manually to make sure that the template was working correctly and to give an idea how it would look.
A few choices I have made:
sklearn.<module>
folders with.gitkeep
files for all the subfolders (sklearn.linear_model
,sklearn.tree
, etc ...) and python files (sklearn.base
,sklearn.calibration
, etc ....), except for private modules or submodules I deemed not relevant (e.g.sklearn.experimental
,sklearn.tests
, etc ...)custom-top-level
folder where you can add custom section. There is an example with dropping setuptools support. For these sections you don't write a bullet point but a section so you need to use the right headline character which is-
currently.Fow now, I am envisioning that you use towncrier to generate a single file from the fragment files but that there is still some manual copy and paste + possibly light edit to add the content to
doc/whats_new/v1.6.rst
but this is up for discussion/refinement.This is inspired from the numpy and astropy setups and I also tried to have a changelog similar to what we currently have (ordered first by submodules then by category).
To try it out:
Should produce something like this:
Any other comments?
There are some places where our changelog is a bit custom (notably "Support for Array API" where the ordering is manually done by functions, then by classes, then Other). This kind of flexibility would not be available through towncrier.
My experience writing (and reading) the towncrier template (that uses jinja2) is not very pleasant so I would avoid doing fancy stuff there unless really necessary.
There are things that I haven't added yet but may be worth it, like adding some description before the bullet points for some sections. We currently do this for Array API and Metadata routing to give a bit of context and a link for more details, see https://scikit-learn.org/dev/whats_new/v1.6.html#support-for-array-api.