Skip to content

Conversation

@JuliaPoo
Copy link
Contributor

@JuliaPoo JuliaPoo commented Jun 3, 2024

It seems #24732 fixed itself, see the following CI runs:

It seems numpy#24732 fixed itself, see the following CI runs:
- github.com/JuliaPoo/numpy/actions/runs/9344912430/job/25716839862
- github.com/JuliaPoo/numpy/actions/runs/9344767573/job/25716459859
@rgommers rgommers added this to the 2.1.0 release milestone Jun 5, 2024
Copy link
Member

@rgommers rgommers left a comment

Choose a reason for hiding this comment

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

That's good news:) Thanks for the PR @JuliaPoo.

Could you please also clean up the definition of USING_CLANG_CL near the top of the file, since it's no longer used?

Also, let's make doubly sure it's gone everywhere - when making that change, could you add [wheel build] to the first line of the commit message? That triggers a full set of wheel builds; those can have different compile flags, so it's useful to check.

I don't really mind not knowing what fixed it - could be changed to SIMD code, move to C17, type promotion changes, a change in the compilers, or just some random fix somewhere.

@mattip
Copy link
Member

mattip commented Jun 5, 2024

There is a similar code pattern in scalar math's test_shift_all_bits. It might be nice to check if that passes now too, perhaps in a separate PR.

JuliaPoo added a commit to JuliaPoo/numpy that referenced this pull request Jun 5, 2024
This is a follow up of numpy#26602 and numpy#24162 where previously
the test fails on Clang-cl. I've tried a few CI runs on my
fork and it seems the test passes now.
@mattip
Copy link
Member

mattip commented Jun 5, 2024

No need to wait for the PyPy wheel build. LGTM.

@mattip mattip merged commit 8207e5c into numpy:main Jun 5, 2024
@mattip
Copy link
Member

mattip commented Jun 5, 2024

Thanks @JuliaPoo

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.

3 participants