adding linewidth argument to pie

[behzadnouri]

as observed here quite inconveniently pie does not accept a linewidth argument.

[sfroid]

@tacaswell
I am a newbie to matplotlib development and I was looking at low hanging fruits to pluck.

I tried my hand at fixing this issue and here is the basic changeset...
sfroid@3b767f6

I have not added any tests or even done elaborate regression tests yet.
Before I do those, I wanted to know if

1. Should this fix be aimed towards the master branch or some other maintenance branch and merged back to master as well? I notice that the "Milestone" field on the right says v1.4.x. Is that the branch I should aim for?
2. What kind of tests should be written to protect this feature from failing again? Should we write an image based test or is a simple test which makes sure that the keyword is accepted without issues enough?

I am following (or going to follow) the development workflow for submitting this fix, but I am new to this, so please bear with me a bit.

Thanks in advance.
sfroid

[tacaswell]

@sfroid Great! We can always use more help.

That change looks reasonable. The next step is to open a Pull Request through the github interface (which is how we manage reviewing and applying). If you include #2899 (or #(any issue number)) in the text of the commit or anywhere on GH it will auto-magically link them.

Please target it at master. Typically new features go on master and bug fixes go on 1.Y.x, but we are in the process of getting the 1.4 release out so its a little messy. The 1.4.x milestone is what I have been tagging things that are not a priority to get done before 1.4.0, but should go on the 1.4.x branch when it exists. (Also to that end, if anything tagged an 1.4.0 and the label 'needs patch' seems interesting please work on those 😄 )

There some house keeping that needs to go with this change as well. You need to add an entry into the CHANGELOG (for devs) and whats_new.rst (for users), add linewidth to the docstring of pie, and probably re-generate the pyplot file.

As for tests, I think this should probably get an image test. I think that pie currently has an image test, use that as a template.

If you are feeling ambitious, my initial thought on this issue was actually to get pie to take **kwargs and pass them through correctly, but I am not sure what 'correctly' is here as we probably also want to be able to pass arguments through to the text.

[sfroid]

@tacaswell
Thanks for the detailed reply.

I have a question regarding the feeling ambitious part, because I guess I am feeling so...
How about implementing it this way (see this commit sfroid@3ceade6)?

[sfroid]

@tacaswell
I added a commit which needs review for the auto generated pyplot.py changes.
sfroid@bd55ab2

Why is "linewidth=linewidth" added to the arguments of pie in pyplot.py (instead of wedgeprops)?
And why have all strings in function arguments in the pyplot.py changed from '' to u'' form? Is there something wrong with my particular setup which is making this happen?

[tacaswell]

You should create a PR, it makes this process easier 😉

[tacaswell]

Closed by #2923