Repository navigation
Add new (2.0) string ufuncs in np.char to UNSUPPORTED_UFUNCS - #15699
Conversation
|
Thank you for your contribution to Astropy! 🌌 This checklist is meant to remind the package maintainers who will review this pull request of some common things to look for.
|
|
@neutrinoceros you're right, this is a separate failure that is not actually covered by CI, see updated comment above; you should be able to reproduce it in your setup by running |
|
Oh I see, indeed that's a separate problem, so I see no harm in keeping our PRs separate. Thank you ! |
|
Indeed I see a difference when running and with this branch but the test fails in all cases ! Do you know what's going on here ? (I rebuilt numpy dev from source right before I wrote this reply) |
|
Yes, just reproduced that after I force-rebuilt numpy from git main to test your branch |
|
Numpy "nightly" isn't really published every Earth night. |
Well, they are the only private ones except one so far: >>> [u for k, u in np._core.umath.__dict__.items() if isinstance(u, np.ufunc) and k.startswith('_')]
[<ufunc '_ones_like'>, <ufunc '_strip_whitespace'>, <ufunc '_lstrip_whitespace'>, <ufunc '_rstrip_whitespace'>, <ufunc '_strip_chars'>, <ufunc '_lstrip_chars'>, <ufunc '_rstrip_chars'>]and |
mhvk
left a comment
There was a problem hiding this comment.
I think it is fine to add also these private ufuncs to the list, especially since we did that already with _ones_likeh, with the logic that if a wrapping function uses those, it will now work (well, will fail appropriately for those string functions!).
So, I'll approve. I won't immediately merge just in case there is an ordering issue, though I'd guess it is best to get this in and rebase the other PR on it. If so, please just go ahead and merge!
p.s. iIf this continues, we can try to be smart and automatically exclude all ufuncs that only works on dtypes "S" and "U".
There was a problem hiding this comment.
Please merge away ! devdeps CI won't go green until we get this and #15691 in (and as we saw it may not be enough still), but as I'll need to rebase mine anyway I'm happy to go in second !
|
Thanks for the reviews! Thought about waiting over the weekend for the _strip functions to get in, but since they are not tested anyway it does not matter. |
|
@dhomeier , please remember milestone and backport label next time. Thanks! |
|
@meeseeksdev backport to v6.0.x |
…UNSUPPORTED_UFUNCS
|
Ow, specifically did not set one initially as I did not recall the backport rules for numpy-2 fixes, but then forgot to go through the checklist, sorry. |
…699-on-v6.0.x Backport PR #15699 on branch v6.0.x (Add new (2.0) string ufuncs in np.char to UNSUPPORTED_UFUNCS)
Description
Additional ufuncs in
np.charnot yet covered byunits.quantity_helpers; added toUNSUPPORTED_UFUNCSas none of them make sense for quantities.Fixes part of #15690 – actually the above were not mentioned there or showed up in any test agains numpy-2.0 since
TestUfuncHelpers.test_coverageis skipped with scipy installed.Tagging @neutrinoceros