Skip to content
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

DOC: Emphasize distinctions between np.copy and ndarray.copy #18741

Merged
merged 3 commits into from
Apr 9, 2021
Merged

DOC: Emphasize distinctions between np.copy and ndarray.copy #18741

merged 3 commits into from
Apr 9, 2021

Conversation

moble
Copy link
Contributor

@moble moble commented Apr 7, 2021

See #15570 and #15685.

@rossbar clarified the documentation for numpy.copy, but ndarray.copy was left unchanged. This distinction recently bit me and one of my users again, which might have been avoided if the distinction were noted in both places.

@melissawm
Copy link
Member

It appears you have included an extra "See Also" section, which sphinx didn't like. You will need to reorganize your changes for that to work.

@moble
Copy link
Contributor Author

moble commented Apr 8, 2021

Thanks @melissawm. Fixed now. I don't understand the codecov error; it's pointing to lines I didn't touch...

Copy link
Member

@melissawm melissawm left a comment

Choose a reason for hiding this comment

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

Thanks @moble !

@moble
Copy link
Contributor Author

moble commented Apr 8, 2021

Thank you!

@moble
Copy link
Contributor Author

moble commented Apr 9, 2021

The same codecov issue that is unrelated to anything I've changed, plus a new shippable error that also looks unrelated — maybe just a connectivity issue?

@moble moble requested a review from melissawm April 9, 2021 14:32
@melissawm
Copy link
Member

LGTM - @rossbar do you have anything to add?

Copy link
Contributor

@rossbar rossbar left a comment

Choose a reason for hiding this comment

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

LGTM as well, thanks @moble & @melissawm

You're right, the coverage/shippable issues are unrelated

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants