Require a problem description in PRs

[jankatins]

A lot of time I can't find out why a change in a commit was made because it either has no proper commit message or the commit message just says what was changed, but not why.

Add text to the PR template to require such a problem description, so that one can comprehend why a change was made even years later.

[jreback]

@JanSchulz the issue templates could be better of course, but they generally don't end up as the actual commit message. They are referenced by number. So if that is what you mean ok.

sorry meant this on your other PR. sure this can be better.

[jankatins]

@jreback as this repo squashes all commits, I think this is the only place where such a problem description makes sense? Or do you have a script which does not take the initial PR message into account?

Just giving an example why I think such a description is nice (I just took the latest commit at the time of writing this; sorry @sinhrks that it's one of yours, a lot of others look the same):

TST: Fix test_coercion for period dtype
xref #13941.

Author: sinhrks <[email protected]>

Closes #14025 from sinhrks/test_coercion_period and squashes the following commits:

b873683 [sinhrks] TST: Fix test_coercion for period dtype

This actually gives no idea what was wrong here and why the new behaviour is the right one. The original commit only has a message ("TST: Fix test_coercion for period dtype"), no description; the PR #14025 has basically no explaination at all; the xrefed issue is actually another PR (#13941) which also has also no clean description what the problem is and only mentioned that it is "split from #13755" which then mentions what has to be done to get that PR merged, but again has no mention what the actual problem is whch that PR tries to fix.

I'm not sure how you know why all this changes are needed and what the context is, but for me as an outside contributor is not clear at all why changes are done, especially when looking at the changes half year or more later. During the Categorical fixes, I spent quite a lot of time trying to understand why changes were done ("Is that behaviour really just by accident or is that by design") and in most of the cases I found only such uninformative commit messages as above :-(

IMO to make decisions/changes more comprehensible (and also to make it easier for newcomers to understand things) it would be nice if changes could be better documented in the actual commits.

[jreback]

@JanSchulz I do this on purpose. Otherwise these comments just clutter the git log. There is the issue reference. Anything else is just superflous.

[jreback]

This one was an accident.

I actually find the PR templates are pretty much useless and always remove them. The purpose is to remind contributors, which maybe helps a little.

[jankatins]

Then this is more a "also remind core contributor to include such a explanation" issue?

I do know that pandas is a really fast moving target and that's probably also because noone really cares about writing extensive commit or PR messages (or docs/comments for internal stuff... :-) ). But from the standpoint of a "not-core-dev", trying to understand commits/PRs later is a real pain :-(

re "commit logs clutter git log": I usually go via the github blame interface and there the commit log is the first thing which comes up but in pandas it's useless, I always have to go to the PR and even referenced issue and reread the whole discussion to (maybe) get an idea what's going on. So adding such a "problem description" at the top of the PR/issue, which explains teh motivation behind a change, would help a lot.

[jreback]

@JanSchulz I do symathize. However, adding more burden to the core devs is not the answer here. Sure, we can push back on the contributors to have a better description. Not entirely sure if the template actually helps with that though (not sure anything helps).......

[sinhrks]

Unrelated to the PR itself, but if I may make excuses, the PR was clear from adjacent one and Travis failure. Though it's correct that we should make backgrounds clear as much as possible... 👍

I'm afraid describing "how" may look to be a little labor for contributors. Most of PRs can be understood from script and comments. otherwise we can request.

how does this change fix that problem

[jankatins]

Not "how", but "why". "How" is clear from the code changes, but why is mostly not.

Basically it's doing more stuff now (=document the why it in the issue/PR) so that later you don't have to reread the discussion or make the research again. Or make it easier for newcomers...

[sinhrks]

OK, then something like this? pls ignore if it is enough clear for natives...

Add description of the problem and why this change is needed (what is the problem, how does this change fix that problem and is the result after the change).

[jreback]

@TomAugspurger @jorisvandenbossche ?

I am leary of adding more text in the PR's. maybe just add some more text in the script/merge-pr script as a reminder?

[TomAugspurger]

No strong opinion from me, mostly because I haven't often run into issues where the intent of a PR was unclear.

[jreback]

I think this is covered by having people merging keep more of the text inside the PR.