Skip to content

BUG: Move legacy check for void printing#24270

Merged
mhvk merged 1 commit intonumpy:mainfrom
seberg:void-scalar-legacy
Jul 27, 2023
Merged

BUG: Move legacy check for void printing#24270
mhvk merged 1 commit intonumpy:mainfrom
seberg:void-scalar-legacy

Conversation

@seberg
Copy link
Member

@seberg seberg commented Jul 27, 2023

The check needs to be in the python path, because also the printing for str() subtly changed.

To be the same as tuples, we should actually print (np.float32(3.), np.int8(1)) for str() (tuples include the repr), while for repr() we include the dtype so we would print similar (but ideally with full precision) to arrays.

This isn't quite ideal, I would be happy to print the full repr in the str() but I guess that might be a bit annoying in practice, so maybe our Numeric types are special enough for now.


@mhvk this should fix the remaining things in astropy. (As said above, no solution seems quite ideal, but it seemed more practical to stay with "numpy builtin types are special for str(void).)

The check needs to be in the python path, because also the printing
for `str()` subtly changed.

To be the same as tuples, we should actually print `(np.float32(3.), np.int8(1))`
for `str()` (tuples include the repr), while for `repr()` we include the dtype
so we would print similar (but ideally with full precision) to arrays.

This isn't quite ideal, I would be happy to print the full repr in the `str()`
but I guess that might be a bit annoying in practice, so maybe our Numeric types
are special enough for now.
Copy link
Contributor

@mhvk mhvk left a comment

Choose a reason for hiding this comment

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

Yes, this looks good!

@mhvk mhvk merged commit 3dd9dba into numpy:main Jul 27, 2023
@seberg seberg deleted the void-scalar-legacy branch July 27, 2023 11:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants