Skip to content

DOC Increase prominence of starting from existing issues #31660

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

betatim
Copy link
Member

@betatim betatim commented Jun 25, 2025

What does this implement/fix? Explain your changes.

This PR makes a few changes to our contributing documentation. The goal is to increase the prominence of the advice to start from known issues and ways of helping out that do not involve writing (unsolicited) code.

I think we can safely state that we are selective (not just somewhat selective) when it comes to new estimators as well as new features. The linked FAQ entry applies to all kinds of code contributions.

Any other comments?

We could probably also work on the PR template to include questions similar to the ones for new features. Maybe in a new PR. We could also make bigger changes to the contrib docs, I was looking at the Numpy guide which feels nice and organised. But again, maybe something for the future

What do people think?

Try to set expectations regarding new features and contributing by
writing code.
@betatim betatim changed the title Tweak contributing docs DOC Increase prominence of starting from existing issues Jun 25, 2025
Copy link

github-actions bot commented Jun 25, 2025

✔️ Linting Passed

All linting checks passed. Your pull request is in excellent shape! ☀️

Generated for commit: f0358af. Link to the linter CI: here

Copy link
Member

@lucyleeow lucyleeow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @betatim ! These is a nice improvement

adding new algorithms, and the best way to contribute and to help the project
is to start working on known issues.
Scikit-learn is :ref:`selective <selectiveness>` when it comes to
adding new algorithms and features. This means the best way to contribute
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
adding new algorithms and features. This means the best way to contribute
adding new algorithms, features and enhancements. This means the best way to contribute

is this too much? 😬

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unsure. I'd argue that an enhancement is a new feature.

@StefanieSenger
Copy link
Contributor

StefanieSenger commented Jun 26, 2025

Thanks a lot, @betatim!

Below, we have the sentence "We are glad to accept any sort of documentation:" (currently line 705) quite prominently. I would think regarding our discussion, that's also not fully true and we should rather hint to that there are of cause conditions?

@lucyleeow
Copy link
Member

Good point. "Thoughtful documentation contributions, not copied directly from a LLM..." 😅

@betatim
Copy link
Member Author

betatim commented Jun 26, 2025

Happy to change it, in particular because I am not quite sure what that sentence is trying to say right now. It feels like documentation isn't a thing you can accept - you can accept contributions to the documentation.

Maybe something like "The project contains many kinds of documentation, contributions to any of these are welcome:"? What do you think?

I would skip the comment about LLMs, feels like going into the details too soon and it is already discussed in the "Automated contributions policy" section.

@StefanieSenger
Copy link
Contributor

StefanieSenger commented Jun 26, 2025

Maybe something like "The project contains many kinds of documentation, contributions to any of these are welcome:"? What do you think?

Hm, I think we want to foreshadow that not any addition to the documentation will be accepted, for expectation management.

Maybe:
"We welcome thoughtful contributions to the documentation and are happy to review additions in the following areas:"
(?)

@betatim
Copy link
Member Author

betatim commented Jun 26, 2025

I added it

Copy link
Contributor

@StefanieSenger StefanieSenger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Its an improvement, thank you @betatim!

I think I would chose much clearer language (see my comments). In reality, it probably doesn't matter so much, because the people who are not considerate are the same who don't read this. 🤷

@StefanieSenger
Copy link
Contributor

@reshamas maybe also has some ideas?

betatim and others added 2 commits June 26, 2025 12:46
Co-authored-by: Stefanie Senger <[email protected]>
@reshamas
Copy link
Member

Adding here the transcript to Andy's video for reference:
https://github.com/data-umbrella/data-umbrella-scikit-learn-sprint/blob/master/1_transcript_ACM_contributing_sklearn.md

slide 14:

I've been with the project for a long time and basically any pull requests that I do will have a long discussion and will undergo many iterations before it gets merged, if it gets merged.

slide 15:

So there's also other ways to contribute then finding an issue and working on them. You can also just fix something in the docs that's unclear. You don't necessarily need to open issue for this, so just like improve the documentation if there's something you don't like about it. Or just open issues. Open issues about unclear docs, about features that you find weird, about examples that are not helpful, about bugs you run into.

Particularly slide 19:

That is because scikit-learn is already quite a mature library and so it's moving quite slowly.

Yeah so if you want to add a major features to scikit-learn, that's probably not something you can do in a day. Adding a new model to scikit-learn is usually something that take many months and it's not something you should try to attempt at the beginning. So really start with something simple and then maybe if you got your first first two pull requests and you can work at adding like a smaller feature but don't count on adding a big feature anytime soon. That is because scikit-learn is already quite a mature library and so it's moving quite slowly. And so it's hard to add anything big or make any big new changes. Also there might be a lot of interesting issues that are not appropriately tagged. So if you're interested in particular topic you can just search the topic on the issue tracker or on the pull requests and see if there's something interesting happening there.

@reshamas
Copy link
Member

@betatim @StefanieSenger
I think this PR is a good start. I have extensive changes I would like to make, but need to map them out (in the same way I did with #31519), but I can do that at a later time.

@betatim
Copy link
Member Author

betatim commented Jun 27, 2025

@reshamas I think taking a "big picture" view of revising thing sis a good idea. Like you said it will require a bit of preparation. Is it ok if we do that outside of this PR and don't wait for it before merging this?

It isn't clear to me what we should do in this PR with the snippets of Andy's talk? To me it feels like a lot of what he said is reflected already in the guide: start small, PRs often go unmerged (no matter who you are), adding a new feature/algorithm is one of the hardest things known to humankind

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants