Reset warning filter during thread-unsafe detection

[ngoldbaum]

This fixes an issue seen in a NumPy PR: numpy/numpy#30605

See e.g. this CI run: https://github.com/numpy/numpy/actions/runs/20823926701/job/59819928182?pr=30605#step:9:185

I spent some time trying to write a test but this is sufficiently complicated that I was unable to get it to work using the pytest testing helpers.

I manually verified that this fixes thread-unsafe detection in NumPy but still preserves the warning in the final output.

@lysnikolaou do you think this is OK, or would you do it in a different way?

We'll need a release out to unblock that NumPy PR.

[lysnikolaou]

While I feel like this is okay and it's actually an improvement, I'm wondering why parsing test_empty_indexing in the linked PR fails. That test function doesn't seem all that complicated and I'm a little worried there's an underlying issue that we might be working around here, especially since there's no test resembling the failure in NumPy.

[ngoldbaum]

That test function doesn't seem all that complicated and I'm a little worried there's an underlying issue that we might be working around here, especially since there's no test resembling the failure in NumPy.

The NumPy PR adds a new deprecation warning when accessing the chararray attribute of np.char. The AST parsing does the same getattr and triggers the new deprecation warning, which is treated as an error by NumPy's pytest configuration. This error then gets caught by the try/except in the thread-unsafe detection code and re-raised as a warning. But because NumPy's test suite treats warnings as errors, the warning is treated like an exception and this crashes the test suite.

My fix is to temporarily reset the warning context state while we're doing thread-unsafe detection. Not sure if there's much else we can do?

[lysnikolaou]

The AST parsing does the same getattr and triggers the new deprecation warning

What I'm saying is that I'm not sure this should be happening. Maybe inspect.getsource does a getattr somewhere? I'll have a look and report back.

[lysnikolaou]

Hmm, it's a getattr in our own code that triggers this, not in ast.parse or inspect.getsource. We could wrap just the call to visitor.visit in the context manager, but I guess we shouldn't be handling any warnings as errors in the error detection code so let's get this in like this. Maybe we could even move the context manager up a level or two in a follow-up.