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: __array__ accepts a dtype argument #18238

Merged
merged 1 commit into from
Jan 28, 2021
Merged

DOC: __array__ accepts a dtype argument #18238

merged 1 commit into from
Jan 28, 2021

Conversation

rsokl
Copy link
Contributor

@rsokl rsokl commented Jan 27, 2021

The documentation for writing custom array containers should indicate that the __array__ interface accepts an optional dtype parameter.

In the present docs, the example class has a bug lurking:

class DiagonalArray:
    def __init__(self, N, value):
        self._N = N
        self._i = value
    def __repr__(self):
        return f"{self.__class__.__name__}(N={self._N}, value={self._i})"
    def __array__(self):
        return self._i * np.eye(self._N)
>>> arr = DiagonalArray(4, 2)
>>> np.asarray(arr, dtype=np.float32)
TypeError: __array__() takes 1 positional argument but 2 were given

@mattip
Copy link
Member

mattip commented Jan 27, 2021

The documentation for the __array__ function is not very descriptive. It describes what the ufunc will not do if __array__ is present. It should describe what the prospective __array__ user is expected to do with self and optional dtype. That would be nice to expand upon here or in a follow-up PR.

@rsokl
Copy link
Contributor Author

rsokl commented Jan 27, 2021

Agreed @mattip - in fact the documentation is so unclear, that I am not even sure what it is saying! Thus I am afraid that I might not be the one to come up with better wording.

@seberg
Copy link
Member

seberg commented Jan 27, 2021

If you are up for it, that would be great anyway, we can always iterate together (no pressure though). The only subtlety I can really think of, is that it should be completely fine to ignore dtype which would just trigger casting in NumPy itself.

@mattip
Copy link
Member

mattip commented Jan 28, 2021

OK, let's put this in as-is, it is a good first step.

@mattip mattip merged commit 2863c52 into numpy:master Jan 28, 2021
@mattip
Copy link
Member

mattip commented Jan 28, 2021

Thanks @rsokl

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