better warning filter for assert_* #6212
Conversation
mathause
force-pushed
the
better_warning_filter
branch
from
e02b28b
to
8525240
Compare
| @@ -29,7 +29,8 @@ def wrapper(*args, **kwargs): | |||
| __tracebackhide__ = True | |||
|
|
|||
| with warnings.catch_warnings(): | |||
| warnings.simplefilter("always") | |||
| # only remove filters that would "error" | |||
| warnings.filters = [f for f in warnings.filters if f[0] != "error"] | |||
There was a problem hiding this comment.
can we rely on warnings.filters? As far as I can tell, that's semi-public at best: the library is very consistent on using _ as a prefix for truly private attributes, but it also doesn't document filters or list it in __all__. However, the standard library doesn't change very often, so I guess it's not too risky...
There was a problem hiding this comment.
Yes, I am not sure either. But I found no "public" way to unset a specific filter. I think even if it breaks at one point it should mainly affect downstream libraries and not users.
The alternatives are
- always warn
- never warn
- remove the decorator
I would prefer the "result" of what I propose here but I also understand if you deem it too dangerous. I had a quick look at the python issues but found nothing in this direction.
There was a problem hiding this comment.
warnings will only change on upgrade to a new version of python, which involves some manual work, anyways (and testing is usually blocked by numba for a few months).
I guess this means it should be safe to use? @pydata/xarray, what do you think?
Edit: if filters was renamed to something else we would get a AttributeError, but that should be fairly easy to handle
Edit2: to be extra safe we could also ask the python core devs for clarification
|
Ok merging this. I think we got some to fix it if this ever causes trouble for a future version of python. cc @headtr1ck |
In #4864 I added a a decorator for the
xarray.testing.assert_*functions to ensure warnings that were to errors (pytest.mark.filterwarnings("error")) do not error inassert_*(see #4760 (comment)). As a solution I addedxarray/xarray/testing.py
Line 32 in 5470d93
However, this is sub-optimal because this now removes all
ignorefilters! As dask stuff only gets evaluated inassert_*filters likewarnings.filterwarnings("ignore", "Mean of empty slice")don't work for dask arrays!I thought of setting
but this could suppress warnings we want to keep.
So now I remove all
"error"warning filters and keep the rest. Note that the original filters get restored afterwith warnings.catch_warnings():. ().I am not sure I expressed myself very clearly... let me know and I can try again. @keewis you had a look at #4864 maybe you can review this PR as well?