-
-
Notifications
You must be signed in to change notification settings - Fork 8k
[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
Conversation
👍 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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
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.
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.
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.
FIX tiny minor fixes on the doc of fill function
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.
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.
If @NelleV is fine with the rendering, I can correct the name for you; do you prefer both names or just one?
I've opened a new pull request with the tiny left over fix (#7268). I am thus closing this one.
Thanks for your contribution!
Hi Nelle,
We have adopted the documentation of "fill" according to numpy style. I hope you like it!
Best,
Zohreh & Bruno
@NelleV