Skip to content

DOC: Clarify __array__ protocol arguments #28226

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

Merged
merged 2 commits into from
Jan 24, 2025
Merged

Conversation

seberg
Copy link
Member

@seberg seberg commented Jan 24, 2025

I was told the current docs are a bit unclear, so trying to make them more clear.
Also, we always were OK with dtype= being ignored, there seems little point in denying that when even pandas does it.

Not sure I quite like the phrasing around "least desirable", but didn't have a better idea (since yes, one should look at all options).

I was told the current docs are a bit unclear, so trying to make them
more clear.
Also, we always were OK with `dtype=` being ignored, there seems little
point in denying that when even `pandas` does it.

Not sure I quite like the phrasing around "least desirable", but didn't
have a better idea (since yes, one should look at all options).
Copy link
Member

@ngoldbaum ngoldbaum left a comment

Choose a reason for hiding this comment

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

Just one small comment, this is a substantial improvement. Out of curiosity, what would happen if someone returned something that isn't an ndarray?

- ``dtype`` is the requested data type of the returned array and is passed
by NumPy positionally (only if requested by the user).
It is acceptable to ignore the ``dtype`` because NumPy will check the
result and cast to ``dtype`` if necessary.
Copy link
Member

Choose a reason for hiding this comment

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

Maybe add "If it is more efficient to coerce the data to the requested dtype without relying on NumPy, you should handle it in your library.", to give people a hint towards the more performant thing.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, was a bit unsure if pointing out the performance impact is worthwhile, but if you lean that way, let me add that sentence.

@seberg
Copy link
Member Author

seberg commented Jan 24, 2025

what would happen if someone returned something that isn't an ndarray?

We raise a ValueError, but don't really feel it's worth noting that.

@ngoldbaum
Copy link
Member

Thanks @seberg

@ngoldbaum ngoldbaum merged commit e338f56 into numpy:main Jan 24, 2025
67 checks passed
@seberg seberg deleted the array-doc branch January 24, 2025 21:55
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.

2 participants