Improve guide to contributing

[jnothman]

I don't think the contributor's guide at present is very easy to read, and it should contain a bit more on certain matters. (At the moment it is mostly focused on technical how-tos.) The main additional things to get across are:

• what belongs in scikit-learn, and what doesn't (and if something doesn't, where can it go)? (This may belong on a separate page)
• more ways to contribute
• what makes a good code contribution
• the pull request process

This needs to be a concise-as-possible one-stop shop for the hundreds of requests to contribute.

I would like to see the guidelines have a shape like the following:

Scikit-learn's scope

This may belong in the contributor's guide, or may belong elsewhere in the documentation (a new "About the project", or in the FAQ). In any case it should be referenced from the contributor's guide.

• well-established algorithms (and occasionally, recent variants of well-established algorithms), to be mostly usable out of the box; in distinction to pylearn2 et al.
• specifically for ML problems whose targets have minimal structure: i.e. binary, regression, multiclass, multilabel and multitask problems from more-or-less flat feature vectors. These all involve (sometimes spare) arrays as input and targets.

Ways to contribute

This is already mostly there, but needs some reordering. It needs to begin with an emphasis on how overtasked the core developer group is, and how important it is to help complete existing highlights a contributor's competence, leading way for larger contributions.

• reporting bugs
• easy issues and how to adopt an issue
• finding and adopting a stagnant pull request (PR): perhaps look for old WIPs
• new features (see scope and discussion of contributions below first; suggest following mailing list)
• documentation
• examples
• testing and improving consistency
• reviewing (an extremely valuable contribution after showing you are familiar with what makes a good contribution)
• spreading the word

How to contribute

• the pull request process: perhaps adopting an issue; WIP status; MRG status; MRG+1; MRG+2; adding to what's new. Refinement and rebasing along the way.
• the ideal code contribution: well-written (clear and reasonably efficient) code consistent with the project's style; tests that ensure it works; understandable, complete docstrings; narrative documentation in the User Guide; examples that highlight interesting features of the algorithm, how to inspect its model, and how to interpret its hyperparameters.
• code management (i.e. git)
• coding guidelines
• API design
• documentation guidelines

[raghavrv]

I'd like to work on this... this will help me learn more about good contribution practices too... (and some complex rebase techniques that we may document in the code management section, if needed)

But is it okay for novices to take this up? If I did could you guide me?

[jnothman]

If I did could you guide me?

Of course!

[raghavrv]

Thanks!! I am starting work on this by 22nd... :) soon :p

[raghavrv]

I am thinking of using the following as reference :

1. Sympy's detailed Development Workflow
2. Sympy's writing documentation guide
3. Scipy's guide
4. Azure contributors guide

Please let me know if you have any additional reference or contradicting view with the above references...

[raghavrv]

I have an additional suggestion...

More often than not several new devs tend to make the same set of mistakes / run into the same set of difficulty... We could add a numbered FAQ like list of common errors / suggestions... ( Apart from the contrib guide which most newbies might not read fully )

So reviewers can save a lot of time by simply saying something like... Refer #34 in that list...

I can prepare that list by scavenging through old PRs and reviews? Do you think such a list would be worth the time spent on it? Please let me know your views... :)

[amueller]

I would maybe do this in a format of a checklist?
Not sure what common mistakes are. Maybe initializing variables in __init__? Not using proper numpydoc format? pep8?

[raghavrv]

For example check this I am sure this must have come across in some other occasssion... Though I am learning a lot from your comments :p , It would be painstaking for you to review multiple PRs with several repeated comments / advices... Instead you could rather simply direct me to a link which has content similar to this :

#1. Use assert_warn_messages wherever appropriate, to check if the message content of the warning matches the expected value... This would be more specific than simply doing assert_warns.

And for this :

#2. Always test that the sphinx html output is rendered correctly whenever you make a change to the rsts / docstrings.

Follow this to check the rendered html output of the docs.

code

You could simply say... "Make sure you follow #2 of that list"

#3. Similarly for this.

I've pointed out my mistakes alone just to be safe... :p but I guess there must be a few more like these around in other PRs?

[amueller]

True, but I would really rather have it as a checklist so that people can go through it before submitting the PR so that we don't have to comment at all ;)
The assert_warn_message is not really a common one, as it is mostly used for common tests and api compatibility things. #2 is certainly common and #3 also. And whitespace before : in docstrings ;)

[raghavrv]

so that people can go through it before submitting the PR so that we don't have to comment at all ;)

Hah :D okay I'll add a checklist instead...

[jnothman]

Any reference, more so a checklist, needs to be concise. It needs to list
the most common issues, or those that are helpful to know in advance of
implementation, but shouldn't get bogged down in detail.

On 6 January 2015 at 06:07, ragv notifications@github.com wrote:

so that people can go through it before submitting the PR so that we
don't have to comment at all ;)

Hah :D okay I'll add a checklist instead...

—
Reply to this email directly or view it on GitHub
#3912 (comment)
.

[raghavrv]

Ok... thanks... I'll keep that in mind... and sorry for the delay in working on this...

[jnothman]

Delay is par for the course in open source!@

On 6 January 2015 at 08:13, ragv notifications@github.com wrote:

Ok... thanks... I'll keep that in mind... and sorry for the delay in
working on this...

—
Reply to this email directly or view it on GitHub
#3912 (comment)
.

[raghavrv]

Have summarized your comment into sections for the proposed new contributions page and added it to wiki for live preview!

Please take a look and feel free to modify anything!

https://github.com/scikit-learn/scikit-learn/wiki/Contributors-Guide

[sotte]

I think the contributing docs should not be spread between CONTRIBUTING.md and doc/developers/index.rst. CONTRIBUTING.md is a thinned out copy of doc/developers/index.rst anyway and DRY.

Maybe just reference the webpage and doc/developers/index.rst from CONTRIBUTING.md.

[amueller]

The CONTRIBUTING.md is shown by github when opening a new issue, though.

[sotte]

That's why I'd put the link to the webpage and doc/developers/index.rst into CONTRIBUTING.md.

For my semi-personal projects I often include README.rst from sphinx into the documentation. This would work for CONTRIBUTING.md/CONTRIBUTING.rst as well.

[raghavrv]

NOTE: Document format for references as described in #4344

[raghavrv]

Hey @jnothman :) Could you take a look at this wiki page and let me know if the structure as defined in "Pages and Sections" seems fine... Once you approve of the structure I'll start the work on this :)

[amueller]

I just realized that our docs don't specify "don't merge master". I think they should.

[hlin117]

It seems that there was a pull request for this issue already. Should this issue be closed?

[raghavrv]

@hlin117 Any issue for which a PR has been raised will automatically / manually be closed as soon as that PR gets merged into master. :)

[amueller]

@rvraghav93 not really true ;) we often forget about that. But this one is not addressed yet.

[devashishd12]

@amueller @rvraghav93 how about adding this with a link to this to contributing.rst under the title "Rebasing and squashing commits". Although I'm not sure whether this should go into that or somewhere else.

[amueller]

we should totally add that. I'm not sure what the state of @rvraghav93's edits is. He had a draft in the wiki, but I think incremental improvements might be easier.
I think it should go to the end of http://scikit-learn.org/dev/developers/contributing.html#how-to-contribute

[devashishd12]

@amueller alright awesome I'll add it as soon as I get a go from @rvraghav93 :) should I start working on the contributor's guide too?

[raghavrv]

Please go ahead. And yes if you want to go ahead with that too. I will try to help if possible as it is a big change.

[amueller]

@jnothman Going through the initial list, I feel like we have all of that. What do you think?

[jnothman]

Yes, I think it's roughly all covered. Let it be closed.