ENH: Add all warnings check to the assert_produces_warnings, and separate messages for each warning. #57222
Conversation
…parate messages for each warning.
rhshadrach added Testing | @rhshadrach Could you review this? Or direct me to someone who can? It's my first contribution here, so I don't know who to ask. |
| I should be able to get to this tomorrow. |
pandas/_testing/_warnings.py Outdated
| type must get encountered. Otherwise, even one expected warning | ||
| results in success. | ||
| .. versionadded:: 3.0.0 |
There was a problem hiding this comment.
This function isn't in the documentation; this line can be removed.
pandas/_testing/_warnings.py Outdated
| match=match, | ||
| check_stacklevel=check_stacklevel, | ||
| ) | ||
| if type(expected_warning) == tuple and must_find_all_warnings: |
There was a problem hiding this comment.
Should use isinstance here I think, hopefully making the cast below unnecessary.
There was a problem hiding this comment.
Thank you, that helped to remove most of the casts.
Jorewin force-pushed the feature/assert-produces-warning-enh branch from b4d9fc0 to 9093962 Compare Jorewin force-pushed the feature/assert-produces-warning-enh branch from 3523b7b to c6d6983 Compare …new API of `assert_produces_warning`
Jorewin force-pushed the feature/assert-produces-warning-enh branch from c6d6983 to 2242796 Compare # Conflicts: # pandas/tests/copy_view/test_chained_assignment_deprecation.py # pandas/tests/frame/methods/test_interpolate.py # pandas/tests/indexing/test_chaining_and_caching.py
| @rhshadrach Could you take another look at this, when you have time? I addressed all change requests and resolved all merge conflicts. The docstring checks seem to still be failing, but it looks unrelated to the changes introduced in this PR. |
# Conflicts: # pandas/tests/io/parser/common/test_inf.py
pandas/_testing/_warnings.py Outdated
| check_stacklevel=check_stacklevel, | ||
| ) | ||
| else: | ||
| expected_warning = cast(type[Warning], expected_warning) |
There was a problem hiding this comment.
This can still be a tuple, no?
There was a problem hiding this comment.
Yes, you're right. I believe this was originally done, to suppress _assert_caught_expected_warning not accepting tuples. I changed that and fixed some issues that arose because of that now.
pandas/_testing/_warnings.py Outdated
| match = ( | ||
| match | ||
| if isinstance(match, tuple) | ||
| else tuple(match for i in range(len(expected_warning))) |
There was a problem hiding this comment.
nit: Use (match,) * len(expected_warning) instead.
| @rhshadrach I just applied a patch that fixed the python 3.9 unit test not passing. With this, this should be ready for review. |
There was a problem hiding this comment.
lgtm, @mroeschke - wouldn't mind a 2nd eye on this if you get the chance.
| This pull request is stale because it has been open for thirty days with no activity. Please update and respond to this comment if you're still interested in working on this. |
| @Jorewin - are you interested in finishing this up? If not, I can add some tests to get it over the finish line. |
| @rhshadrach If you want to, then go ahead. I just didn't have much time lately. |
| @mroeschke - tests added for various combinations. Let me know if you see any others to add. |
| Thanks @Jorewin and @rhshadrach |
…rate messages for each warning. (pandas-dev#57222) * ENH: Add all warnings check to the `assert_produces_warnings`, and separate messages for each warning. * Fix typing errors * Fix typing errors * Remove unnecessary documentation * Change `assert_produces_warning behavior` to check for all warnings by default * Refactor typing * Fix tests expecting a Warning that is not raised * Adjust `raises_chained_assignment_error` and its dependencies to the new API of `assert_produces_warning` * Fix `_assert_caught_expected_warning` typing not including tuple of warnings * fixup! Refactor typing * fixup! Fix `_assert_caught_expected_warning` typing not including tuple of warnings * fixup! Fix `_assert_caught_expected_warning` typing not including tuple of warnings * Add tests --------- Co-authored-by: Richard Shadrach <rhshadrach@gmail.com>
3 participants
The function in question gets used in multiple places, that's why I felt it would be safer to preserve current behavior and API, and add an additional, optional parameter, which alters the behavior, enabling checking if all passed warning types get detected.
doc/source/whatsnew/vX.X.X.rstfile if fixing a bug or adding a new feature.