Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

[WIP] documentation for def fill() #7034

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

Closed

Conversation

Copy link
Contributor

@ZohrehPythonSchool ZohrehPythonSchool commented Sep 5, 2016

Hi Nelle,

We have adopted the documentation of "fill" according to numpy style. I hope you like it!

Best,
Zohreh & Bruno

@NelleV

@tacaswell tacaswell added this to the 2.0.1 (next bug fix release) milestone Sep 5, 2016
Copy link
Member

👍 to the doc fixup, however this put in a mix tabs and spaces in the source. Can you please replace all of the tabs with 4 spaces?


Parameters
----------
args : a variable length argument, allowing for multiple
Copy link
Member

@tacaswell tacaswell Sep 5, 2016

Choose a reason for hiding this comment

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

This wrapping is a bit funny.

Copy link
Member

@QuLogic QuLogic Sep 6, 2016

Choose a reason for hiding this comment

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

args is not really the name of a parameter; it should be x and y, but I'm not sure how to indicate that any number of them can be passed.

An arbitrary number of *x*, *y*, *color* groups can be specified::
ax.fill(x1, y1, 'g', x2, y2, 'r')

kwargs : The *closed* kwarg will close the polygon when *True* (default).
Copy link
Member

@QuLogic QuLogic Sep 6, 2016

Choose a reason for hiding this comment

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

Keyword arguments should got in a "Keyword Arguments" section, listed individually.

Copy link
Member

Choose a reason for hiding this comment

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

There is no keyword arguments in numpydoc, but the kwargs should indeed be moved to a "Other parameters" section.

Copy link
Member

QuLogic commented Sep 6, 2016

It appears you have not set the config for your git client. The committer in your commit is attributed to "Your Name you@example.com", which is probably not what you want. Note that the author does have correct attribution.

Copy link
Member

NelleV commented Sep 14, 2016

I did a PR on your branch to fix the small problems left. If you merge the PR, I think we'll be good on this contribution.

@NelleV NelleV changed the title (削除) documentation for def fill() (削除ここまで) (追記) [WIP] documentation for def fill() (追記ここまで) Sep 14, 2016
Copy link
Member

QuLogic commented Sep 14, 2016

The first commit's attribution includes your first name, but the other two do not. I'm not sure if you want that, but if you are not comfortable with rebasing and changing it, we can perhaps do that for you before merging.

Copy link
Contributor Author

Hi Nell,

Sorry that it took me so long! I think it should be fine now!

Best,
Zohreh

On 14 Sep 2016, at 04:11, Nelle Varoquaux <notifications@github.com mailto:notifications@github.com> wrote:

I did a PR on your branch to fix the small problems left. If you merge the PR, I think we'll be good on this contribution.


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHubhttps://github.com//pull/7034#issuecomment-246893680, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AU_BC4Sb8ObEGGvH-NwuWumoIkdjglM2ks5qp2XwgaJpZM4J1Kdd.

Copy link
Contributor Author

I’m really sorry, I missed this email and did the merge. Is it possible to change my name now? If so, please do feel free to do that.

On 14 Sep 2016, at 04:52, Elliott Sales de Andrade <notifications@github.com mailto:notifications@github.com> wrote:

The first commit's attribution includes your first name, but the other two do not. I'm not sure if you want that, but if you are not comfortable with rebasing and changing it, we can perhaps do that for you before merging.


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHubhttps://github.com//pull/7034#issuecomment-246899034, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AU_BC9D7N3yw2Px13ukGXGJQ31IOG0-bks5qp29igaJpZM4J1Kdd.

Copy link
Member

QuLogic commented Sep 14, 2016

If @NelleV is fine with the rendering, I can correct the name for you; do you prefer both names or just one?

Copy link
Member

NelleV commented Oct 13, 2016
edited by QuLogic
Loading

I've opened a new pull request with the tiny left over fix (#7268). I am thus closing this one.

Thanks for your contribution!

@QuLogic QuLogic modified the milestones: 2.0.1 (next bug fix release), unassigned Dec 7, 2016
@QuLogic QuLogic removed this from the 2.0.1 (next bug fix release) milestone Dec 7, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /