Skip to content

better warning filter for assert_* #6212

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Sep 5, 2022
Merged

better warning filter for assert_* #6212

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
1df63cc
better warning filter for assert_*
mathause Jan 31, 2022
8525240
better comments
mathause Jan 31, 2022
c069de8
fix comment
mathause Jan 31, 2022
f510837
Merge branch 'main' into better_warning_filter
mathause Jan 31, 2022
6ad1ec4
Merge branch 'main' into better_warning_filter
mathause Jul 15, 2022
66d2111
Merge branch 'main' into better_warning_filter
mathause Aug 26, 2022
06fc8c9
Merge branch 'main' into better_warning_filter
mathause Sep 5, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion xarray/testing.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,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"]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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...

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Collaborator

@keewis keewis Feb 3, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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


return func(*args, **kwargs)

Expand Down
13 changes: 13 additions & 0 deletions xarray/tests/test_testing.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,3 +164,16 @@ def __array__(self):
getattr(xr.testing, func)(a, b)

assert len(w) > 0

# ensure warnings still raise outside of assert_*
with pytest.raises(UserWarning):
warnings.warn("test")

# ensure warnings stay ignored in assert_*
with warnings.catch_warnings(record=True) as w:
# ignore warnings
warnings.filterwarnings("ignore")
with pytest.raises(AssertionError):
getattr(xr.testing, func)(a, b)

assert len(w) == 0