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

BUG: Fix C types in scalartypes #24240

Merged
merged 1 commit into from
Jul 25, 2023
Merged

Conversation

thesamesam
Copy link
Contributor

#23746 introduced a fast path for scalar int conversions, but the map between Python types and C types was subtly wrong.

This fixes tests on at least ppc32 (big-endian).

Many thanks to Sebastian Berg for debugging this with me and pointing out what needed to be fixed.

Closes #24239.

Fixes: 81caed6

@thesamesam
Copy link
Contributor Author

thesamesam commented Jul 23, 2023

cc @seberg @mattip

Tested on ppc32 + amd64, I'll check some other platforms too. This gets us down to 1 failure on ppc (with some existing skips I'm not counting: FAILED f2py/tests/test_kind.py::TestKind::test_int - AssertionError: selectedintkind(19): expected 16 but got -1).

@charris charris added the 09 - Backport-Candidate PRs tagged should be backported label Jul 23, 2023
@@ -301,10 +301,10 @@ genint_type_str(PyObject *self)
item = PyLong_FromUnsignedLong(*(uint32_t *)val);
Copy link
Member

Choose a reason for hiding this comment

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

These and above are also wrong (or maybe just badly named). Probably makes sense to just use npy_<name>, although they are the same anyway.
(I suppose, in practice we probably don't support any platform for which all but long aren't correct.)

@seberg seberg added this to the 1.25.2 release milestone Jul 24, 2023
numpy#23746 introduced a fast path for scalar
int conversions, but the map between Python types and C types was subtly
wrong.

This fixes tests on at least ppc32 (big-endian).

Many thanks to Sebastian Berg for debugging this with me and pointing out
what needed to be fixed.

Closes numpy#24239.

Fixes: 81caed6
Copy link
Member

@seberg seberg left a comment

Choose a reason for hiding this comment

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

Force pushed the larger fix, good to go now.

Copy link
Member

@mattip mattip left a comment

Choose a reason for hiding this comment

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

Thanks

@seberg seberg merged commit 4a3cd64 into numpy:main Jul 25, 2023
59 checks passed
@thesamesam thesamesam deleted the fix-scalararray-types branch July 25, 2023 17:35
@thesamesam
Copy link
Contributor Author

Force pushed the larger fix, good to go now.

Thanks! Sorry I hadn't got back to it yet, appreciate you finishing it off.

@charris charris removed the 09 - Backport-Candidate PRs tagged should be backported label Jul 30, 2023
@charris charris removed this from the 1.25.2 release milestone Jul 30, 2023
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.

BUG: Unusual test failures (whitespace-related?) on ppc32 with numpy-1.25.1
4 participants