Bug: assert_produces_warning(None) not raising AssertionError with warning #38626

mzeitlin11 · 2020-12-21T20:14:12Z

closes BUG: tm.assert_produces_warning() doesn't work with expected_warning=None or False #38620
tests added / passed
passes black pandas
passes git diff upstream/master -u -- "*.py" | flake8 --diff

mzeitlin11 · 2020-12-21T20:14:56Z

These changes cause 23 test failures from tm.assert_produces_warning(None) now actually raising an AssertionError if a warning is given. What is the right approach here? Should those tests just be xfailed and an issue made to look into those warnings?

arw2019 · 2020-12-21T20:23:04Z

pandas/_testing.py

@@ -2724,11 +2724,10 @@ class for all warnings. To check that no warning is returned,
        extra_warnings = []

        for actual_warning in w:


isn't this code functionally the same as existing?

I thought if expected_warning is False or None, then extra_warnings does not get appended to because of the continue. So the check at the end for raising on extra warnings doesn't get triggered.

Current implementation will ignore the else clause below, which consolidates extra warnings. I see that I messed that up in one of my previous PRs. This PR reverts that error.
Probably, if some functions are extracted, that would improve the readability of the logic (maybe in a separate PR).

The added tests do raise on master as well, so behavior for them has changed

ivanovmg

Looks like this fixes the issue reported.
Let us take a look at the failing tests.

ivanovmg · 2020-12-21T20:25:13Z

pandas/_testing.py

-            if not expected_warning:
-                continue
-
            expected_warning = cast(Type[Warning], expected_warning)
-            if issubclass(actual_warning.category, expected_warning):
+            if expected_warning and issubclass(
+                actual_warning.category, expected_warning
+            ):


Yes, this is a good catch.

ivanovmg · 2020-12-21T20:27:34Z

pandas/tests/util/test_assert_produces_warning.py

@@ -152,3 +152,20 @@ def test_right_category_wrong_match_raises(pair_different_warnings):
        with tm.assert_produces_warning(target_category, match=r"^Match this"):
            warnings.warn("Do not match it", target_category)
            warnings.warn("Match this", other_category)
+
+
+@pytest.mark.parametrize("false_or_none", [False, None])


Good tests to prevent the issue from happening!

jreback · 2020-12-21T23:37:38Z

wow

FAILED pandas/tests/arithmetic/test_timedelta64.py::TestTimedelta64ArithmeticUnsorted::test_tda_add_dt64_object_array[DataFrame-'UTC']
FAILED pandas/tests/arithmetic/test_timedelta64.py::TestTimedelta64ArithmeticUnsorted::test_tda_add_dt64_object_array[DataFrame-'US/Eastern']
FAILED pandas/tests/arithmetic/test_timedelta64.py::TestTimedelta64ArithmeticUnsorted::test_tda_add_dt64_object_array[DataFrame-'Asia/Tokyo']
FAILED pandas/tests/arithmetic/test_timedelta64.py::TestTimedelta64ArithmeticUnsorted::test_tda_add_dt64_object_array[DataFrame-'dateutil/US/Pacific']
FAILED pandas/tests/arithmetic/test_timedelta64.py::TestTimedelta64ArithmeticUnsorted::test_tda_add_dt64_object_array[DataFrame-'dateutil/Asia/Singapore']
FAILED pandas/tests/arithmetic/test_timedelta64.py::TestTimedelta64ArithmeticUnsorted::test_tda_add_dt64_object_array[DataFrame-'+01:15']
FAILED pandas/tests/arithmetic/test_timedelta64.py::TestTimedelta64ArithmeticUnsorted::test_tda_add_dt64_object_array[DataFrame-'-02:15']
FAILED pandas/tests/arithmetic/test_timedelta64.py::TestTimedelta64ArithmeticUnsorted::test_tda_add_dt64_object_array[DataFrame-'UTC+01:15']
FAILED pandas/tests/arithmetic/test_timedelta64.py::TestTimedelta64ArithmeticUnsorted::test_tda_add_dt64_object_array[DataFrame-'UTC-02:15']
FAILED pandas/tests/arithmetic/test_timedelta64.py::TestTimedelta64ArithmeticUnsorted::test_tda_add_dt64_object_array[DataFrame-tzutc()]
FAILED pandas/tests/arithmetic/test_timedelta64.py::TestTimedelta64ArithmeticUnsorted::test_tda_add_dt64_object_array[DataFrame-tzlocal()]
FAILED pandas/tests/arithmetic/test_timedelta64.py::TestTimedelta64ArithmeticUnsorted::test_tda_add_dt64_object_array[DataFrame-pytz.FixedOffset(300)]
FAILED pandas/tests/arithmetic/test_timedelta64.py::TestTimedelta64ArithmeticUnsorted::test_tda_add_dt64_object_array[DataFrame-<UTC>]
FAILED pandas/tests/arithmetic/test_timedelta64.py::TestTimedelta64ArithmeticUnsorted::test_tda_add_dt64_object_array[DataFrame-pytz.FixedOffset(-300)]
FAILED pandas/tests/arithmetic/test_timedelta64.py::TestTimedelta64ArithmeticUnsorted::test_tda_add_dt64_object_array[DataFrame-datetime.timezone.utc]
FAILED pandas/tests/arithmetic/test_timedelta64.py::TestTimedelta64ArithmeticUnsorted::test_tda_add_dt64_object_array[DataFrame-datetime.timezone(datetime.timedelta(seconds=3600))]
FAILED pandas/tests/arithmetic/test_timedelta64.py::TestTimedelta64ArithmeticUnsorted::test_tda_add_dt64_object_array[DataFrame-datetime.timezone(datetime.timedelta(days=-1, seconds=82800), 'foo')]
FAILED pandas/tests/frame/test_arithmetic.py::TestFrameArithmetic::test_binop_other[add-True-bool]
FAILED pandas/tests/frame/test_arithmetic.py::TestFrameArithmetic::test_binop_other[mul-True-bool]
FAILED pandas/tests/indexes/test_common.py::TestCommon::test_asi8_deprecation[int]
FAILED pandas/tests/indexes/test_common.py::TestCommon::test_asi8_deprecation[uint]
FAILED pandas/tests/indexes/test_common.py::TestCommon::test_asi8_deprecation[range]
FAILED pandas/tests/indexes/test_common.py::TestCommon::test_asi8_deprecation[repeats]

ok these at least should be straightforward to fix

mzeitlin11 · 2020-12-22T01:44:22Z

Bottom 4 failures can be fixed by removing Int64Index and UInt64Index from the tuple in

pandas/pandas/tests/indexes/test_common.py

Lines 362 to 363 in ca52e39

    
           if isinstance( 
        
               index, (Int64Index, UInt64Index, DatetimeIndex, TimedeltaIndex, PeriodIndex)

Looks like this deprecation came from #37877, this removal would make sense if those indexes should give a FutureWarning with .asi8 (as currently happens in master). cc @jbrockmendel

jreback · 2020-12-22T02:04:34Z

Bottom 4 failures can be fixed by removing Int64Index and UInt64Index from the tuple in

pandas/pandas/tests/indexes/test_common.py

Lines 362 to 363 in ca52e39

if isinstance(

index, (Int64Index, UInt64Index, DatetimeIndex, TimedeltaIndex, PeriodIndex)

Looks like this deprecation came from #37877, this removal would make sense if those indexes should give a FutureWarning with .asi8 (as currently happens in master). cc @jbrockmendel

can also just xfail these 3 tests and open an issue as well.

…es_warning

jreback · 2020-12-22T02:50:32Z

pandas/tests/arithmetic/test_timedelta64.py

@@ -543,6 +543,7 @@ def test_tda_add_sub_index(self):
        expected = tdi - tdi
        tm.assert_index_equal(result, expected)

+    @pytest.mark.xfail(reason="GH38630", strict=False)


is there a reason you are passing strict? we default to true - eg if these are fixed we want the tests to fail (as a hint to remove the xfail)

Problem I ran into is that some parameterizations actually pass, so strict=True fails for these. Couldn't figure out a way to xfail only the failing combinations because the parameterizations are complex (2 defined elsewhere in fixtures, 1 uses multiple calls to pytest.mark.parametrize).

ok that's fine

ping on green

Seeing 2 failures where pandas/tests/io/parser/test_common.py::test_chunks_have_consistent_numerical_type[python] gives unexpected ResourceWarning.

Do you know if this warning occurs consistently? Should something in the test be modified to handle a potential ResourceWarning? Or just another xfail case?

we ar trying to track these cases down as something is leaking

if they r causing actual failures then ok to xfail (and list these in the associated issue with checkboxes)

mzeitlin11 · 2020-12-22T05:33:00Z

Green except Travis

jreback · 2020-12-22T13:40:47Z

thanks @mzeitlin11 keep em coming!

…rning (pandas-dev#38626)

mzeitlin11 added 5 commits December 21, 2020 14:34

BUG: assert_produces_warning() with None or False doesn't raise

724a2af

Remove commented line

0a625f3

Honor extra warnings arg

2e1d58a

Add test for None with false raise on extra

aed3465

Refactor added tests

c82b859

mzeitlin11 marked this pull request as draft December 21, 2020 20:14

arw2019 reviewed Dec 21, 2020

View reviewed changes

ivanovmg reviewed Dec 21, 2020

View reviewed changes

arw2019 added the Testing pandas testing functions or related to the test suite label Dec 21, 2020

Merge remote-tracking branch 'upstream/master' into bug/assert_produc…

1938ad9

…es_warning

mzeitlin11 mentioned this pull request Dec 22, 2020

TST/BUG: Unexpected warnings causing failures on master with fixed assert_produces_warnings(None) behavior #38630

Closed

4 tasks

xfail failing tests

6f3c8ff

jreback reviewed Dec 22, 2020

View reviewed changes

mzeitlin11 marked this pull request as ready for review December 22, 2020 03:36

xfail parser test

8193ea1

jreback added this to the 1.3 milestone Dec 22, 2020

jreback merged commit 0519914 into pandas-dev:master Dec 22, 2020

mzeitlin11 deleted the bug/assert_produces_warning branch December 22, 2020 14:31

luckyvs1 pushed a commit to luckyvs1/pandas that referenced this pull request Jan 20, 2021

Bug: assert_produces_warning(None) not raising AssertionError with wa…

afa10ee

…rning (pandas-dev#38626)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bug: assert_produces_warning(None) not raising AssertionError with warning #38626

Bug: assert_produces_warning(None) not raising AssertionError with warning #38626

mzeitlin11 commented Dec 21, 2020

mzeitlin11 commented Dec 21, 2020

arw2019 Dec 21, 2020

mzeitlin11 Dec 21, 2020

ivanovmg Dec 21, 2020 •

edited

mzeitlin11 Dec 21, 2020

ivanovmg left a comment

ivanovmg Dec 21, 2020

ivanovmg Dec 21, 2020

jreback commented Dec 21, 2020

mzeitlin11 commented Dec 22, 2020

jreback commented Dec 22, 2020

jreback Dec 22, 2020

mzeitlin11 Dec 22, 2020

jreback Dec 22, 2020

mzeitlin11 Dec 22, 2020

jreback Dec 22, 2020

mzeitlin11 commented Dec 22, 2020

jreback commented Dec 22, 2020

		@@ -2724,11 +2724,10 @@ class for all warnings. To check that no warning is returned,
		extra_warnings = []

		for actual_warning in w:

Bug: assert_produces_warning(None) not raising AssertionError with warning #38626

Bug: assert_produces_warning(None) not raising AssertionError with warning #38626

Conversation

mzeitlin11 commented Dec 21, 2020

mzeitlin11 commented Dec 21, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ivanovmg Dec 21, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ivanovmg left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jreback commented Dec 21, 2020

mzeitlin11 commented Dec 22, 2020

jreback commented Dec 22, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mzeitlin11 commented Dec 22, 2020

jreback commented Dec 22, 2020

ivanovmg Dec 21, 2020 •

edited