Repository navigation
BUG: Fix interaction between warnings and the promotion cache - #31617
Conversation
ngoldbaum
added
00 - Bug
component: numpy._core
09 - Backport-Candidate
There was a problem hiding this comment.
The code looks fine and since we only fetch the context var when we would warn (and it is otherwise changed in promotion) it should also be fine.
That said, while I did think that a helper function would be far better, is it really worth to go through this trouble to avoid one duplicate warning here?
I guess some test order changes made them appear more in the test run?
| NPY_NO_EXPORT int | ||
| npy_end_legacy_resolver_promotion(PyObject *token) | ||
| { | ||
| int result = PyContextVar_Reset( |
There was a problem hiding this comment.
Ah, seems this works with an error in progress in practice (since I think we have debug python in CI).
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
|
Doesn't look like a blocker, so pushing off to 2.5.1 |
|
I'm going to merge with |
Well that's fun. |
|
@ngoldbaum I think there was something missing for this backport. It was a fairly complicated backport, I wonder if the problem was actually present in 2.5.0? |
|
Yeah, you are right. It seems that this path had missed 2.5 (which doesn't seem like a big issue to me, probably have to take this a bit slow anyway). |
|
Oops, I'm sorry, I thought this had landed in 2.5! |
charris
added
09 - Backport-Candidate
Fixes #31758
This fixes an issue that was introduced recently that causes duplicate warnings to be emitted for the integer/timedelta deprecation.
If accessing the promotion cache leads to a miss, the deprecation is emitted to fill the cache and then emitted again during type resolution.
This means warnings would sometimes be emitted twice.
The fix is to disable warnings when the promotion cache is filled. This happens using a new context variable. This is all happening outside the hot path for these operations so I'm not worried about performance.
I used Claude Fable to help investigate and fix this issue.
Ping @riku-sakamoto and @may12day since you were both involved in the PR that added these warnings.