BUG: Fix TestStringDiscovery float casting warning on FreeBSD #28331
Conversation
e9f1d98 to
31c0e1e
Compare
|
Sorry to keep giving you pushback like this, but I want to strongly discourage you from "fixing" bugs by ignoring errors or warning in the tests without first doing some investigation to justify doing so. This just started failing recently, it's likely due to a change in freebsd or in numpy. We should figure that out. If CI has a failing job for a little while that's OK. |
Thanks @ngoldbaum , completely agree with your point. |
ad0bdd9 to
80986de
Compare
80986de to
97fcf3e
Compare
|
Update patch and commit msg. The current CI failure Linux Qemu tests / loongarch64 (pull_request) has nothing to do with this patch. |
Did you do the investigation to justify disabling the warning? I’m reading into your motivations a bit in what follows so apologies if I’m assuming incorrectly. Note that the CI does not need to be all green on your other PR to merge it, so pushing on this isn’t helping your other PR or helping to unblock turning on aarch64 CI. Please have some patience. Ignoring me is making me less likely to look at your PRs. |
|
Hi @ngoldbaum
Yes, I have tried to investigate the following part of the code, but based on my knowledge, I did not find any specific issues related to FreeBSD: https://github.com/numpy/numpy/blob/e20317a43d3714f9085ad959f68c1ba6bc998fcd/numpy/_core/src/multiarray/stringdtype/casts.cpp#L1191C1-L1256C3
Agree with you on this. Initially, I applied a patch to ignore runtime warnings across all platforms using the following code (which is not good at all): Now, I have the patch to ignore warnings for a specific test on FreeBSD, focusing on a single warning related to float-to-string conversions only (may be a good improvement from the initial approach). My purpose of this patch is not to unblock my other PRs. Instead, I am focused on understanding how the NumPy codebase operates, particularly how NumPy tests and handles warnings. I hope this clarifies my motivation for this patch. |
|
I am going to close this patch. Further investigation may be required. Thanks @ngoldbaum |
|
@ngoldbaum checking the approach here: why wouldn't we merge this? It doesn't close the issue, but CI has been red for days, so merging this PR to turn it green and then defer detailed investigation of this pretty niche issue till later seems like the right approach to me. |
|
I was hoping someone would look more closely to understand what’s happening inside a freebsd VM. I’ll try to do that tomorrow. I was asked to re-review the PR and didn’t see any response besides just rebasing the original commit and an ask to re-review. I’m reopening and will merge this tomorrow if I can’t figure out a better explanation for what’s happening besides “freebsd changed something”. |
|
I think opening a new issue and merging this PR is the best way forward. |
|
Closing this, #28358 fixes it. |
Fixes #28329
The
test_nested_arrays_stringlengthtest intest_array_coercion.pywas failing on FreeBSD due to a platform-specific RuntimeWarning when casting float objects to string dtype. This PR modifies the test to properly handle and verify this warning.Changes:
cc: @ngoldbaum @rgommers @charris