Remove return values for asserts #25462

samuelsinayoko · 2019-02-27T18:54:51Z

Unless they are context managers.

closes assert_*_equal issues #25135
tests added / passed (not applicable)
passes git diff upstream/master -u -- "*.py" | flake8 --diff
whatsnew entry

Unless they are context managers.

WillAyd · 2019-02-27T19:28:51Z

Looks like a slew of test changes as a result of code like assert tm.assert_X_equal which should get updated here.

If you test locally first should make it easier to find up front

samuelsinayoko · 2019-02-27T20:40:21Z

OK am hoping to fix the tests shortly.

assert either raises or doesn't. If it doesn't tm.assert_* returns None, and there's no need to assert XXX is None.

codecov · 2019-03-03T14:26:06Z

Codecov Report

Merging #25462 into master will decrease coverage by 50.01%.
The diff coverage is 50%.

@@             Coverage Diff             @@
##           master   #25462       +/-   ##
===========================================
- Coverage   91.74%   41.72%   -50.02%     
===========================================
  Files         173      173               
  Lines       52923    52922        -1     
===========================================
- Hits        48554    22084    -26470     
- Misses       4369    30838    +26469

Flag	Coverage Δ
#multiple	`?`
#single	`41.72% <50%> (-0.01%)`	⬇️

Impacted Files	Coverage Δ
pandas/util/testing.py	`51.57% <50%> (-36%)`	⬇️
pandas/io/formats/latex.py	`0% <0%> (-100%)`	⬇️
pandas/core/categorical.py	`0% <0%> (-100%)`	⬇️
pandas/io/sas/sas_constants.py	`0% <0%> (-100%)`	⬇️
pandas/tseries/plotting.py	`0% <0%> (-100%)`	⬇️
pandas/tseries/converter.py	`0% <0%> (-100%)`	⬇️
pandas/io/formats/html.py	`0% <0%> (-99.35%)`	⬇️
pandas/core/groupby/categorical.py	`0% <0%> (-95.46%)`	⬇️
pandas/io/sas/sas7bdat.py	`0% <0%> (-91.17%)`	⬇️
pandas/io/sas/sas_xport.py	`0% <0%> (-90.15%)`	⬇️
... and 131 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fe1654f...220052a. Read the comment docs.

codecov · 2019-03-03T14:26:06Z

Codecov Report

Merging #25462 into master will decrease coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master   #25462      +/-   ##
==========================================
- Coverage   91.98%   91.98%   -0.01%     
==========================================
  Files         175      175              
  Lines       52374    52373       -1     
==========================================
- Hits        48178    48173       -5     
- Misses       4196     4200       +4

Flag	Coverage Δ
#multiple	`90.53% <100%> (-0.01%)`	⬇️
#single	`40.72% <44.44%> (-0.15%)`	⬇️

Impacted Files	Coverage Δ
pandas/util/testing.py	`90.6% <100%> (-0.01%)`	⬇️
pandas/io/gbq.py	`78.94% <0%> (-10.53%)`	⬇️
pandas/core/frame.py	`96.9% <0%> (-0.12%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 94c8c94...f4497fd. Read the comment docs.

gfyoung · 2019-03-04T03:33:23Z

@samuelsinayoko : This looks much better! I still see a handful of return assert_* statements in that file (assert_almost_equal and assert_series_equal implementations contain the remainder). Can those be removed as well?

samuelsinayoko · 2019-03-04T07:52:18Z

@samuelsinayoko : This looks much better! I still see a handful of return assert_* statements in that file (assert_almost_equal and assert_series_equal implementations contain the remainder). Can those be removed as well?

done in 60ec5ee

gfyoung · 2019-03-04T08:48:09Z

@samuelsinayoko : I think you only removed the ones in assert_almost_equal. There are still two in assert_series_equal. Were you able to remove those?

samuelsinayoko · 2019-03-04T09:47:33Z

Ha sorry! Will update the PR today.

…

Sent from my iPhone

On 4 Mar 2019, at 08:48, gfyoung ***@***.***> wrote: @samuelsinayoko : I think you only removed the ones in assert_almost_equal. There are still two in assert_series_equal. Were you able to remove those? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

pandas/util/testing.py

WillAyd

lgtm @jreback

jreback · 2019-03-19T13:04:49Z

pandas/tests/series/test_constructors.py

@@ -1009,7 +1009,7 @@ def test_fromDict(self):
        data = {'a': 0, 'b': 1, 'c': 2, 'd': 3}

        series = Series(data)
-        assert tm.is_sorted(series.index)
+        tm.is_sorted(series.index)


can you rename this routine to tm.assert_is_sorted

pandas/util/testing.py

jreback · 2019-03-19T13:05:40Z

pandas/util/testing.py


    elif (is_extension_array_dtype(left) and not is_categorical_dtype(left) and
          is_extension_array_dtype(right) and not is_categorical_dtype(right)):
-        return assert_extension_array_equal(left.array, right.array)
+        assert_extension_array_equal(left.array, right.array)
+        return


same don't need a return

I tried this earlier but some of the tests failed so I added the returns back.

See 091a00e (this was in response to an earlier PR comment)

jreback · 2019-03-26T00:27:49Z

can you merge master and update

samuelsinayoko · 2019-03-26T08:08:29Z

@jreback have merged master and updated the branch. Don't think I can leave out the return statements as you suggested because that breaks the buil (search back for the build of 091a00e above)

samuelsinayoko · 2019-03-26T14:13:57Z

OK I'll give this another go.

…

On Tue, 26 Mar 2019 at 11:52, Jeff Reback ***@***.***> wrote: @samuelsinayoko <https://github.com/samuelsinayoko> you may need to fix some tests that are actually asserting the result of this assert, but I want to remove the return values — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#25462 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AQuwvoIm4H69yV_3a7pOHtrxIzmje36Rks5vagntgaJpZM4bVN-a> .

-- *Sam Sinayoko* | Data Scientist *BMLL Technologies Ltd.* 10th Floor – Portland House Bressenden Place London SW1E 5RS +44(0)20 3828 9020

jreback · 2019-04-05T00:51:06Z

can you merge master and update

samuelsinayoko · 2019-04-05T07:25:05Z

Apologies, haven't found the time to come back to this as work's been crazy for the past couple of weeks but will update the PR over the weekend.

…

On Fri, 5 Apr 2019 at 01:51, Jeff Reback ***@***.***> wrote: can you merge master and update — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#25462 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AQuwvuUm4RYJbZPrU-0idZAykUR1dzw5ks5vdp4SgaJpZM4bVN-a> .

-- *Sam Sinayoko* | Data Scientist *BMLL Technologies Ltd.* 10th Floor – Portland House Bressenden Place London SW1E 5RS +44(0)20 3828 9020

jreback · 2019-04-07T21:27:48Z

pandas/util/testing.py


    elif (is_extension_array_dtype(left) and not is_categorical_dtype(left) and
          is_extension_array_dtype(right) and not is_categorical_dtype(right)):
-        return assert_extension_array_equal(left.array, right.array)
+        assert_extension_array_equal(left.array, right.array)
+        return


this just returns None which is the same as if you leave the return out, pls remove

jreback · 2019-04-20T16:58:17Z

can you merge master

WillAyd · 2019-04-30T01:44:49Z

Can you merge and fix up conflicts with master? I think this will be a precursor to #26234

samuelsinayoko · 2019-04-30T06:54:57Z

Sure. Will do tonight.

…

Sent from my iPhone

On 30 Apr 2019, at 02:45, William Ayd ***@***.***> wrote: Can you merge and fix up conflicts with master? I think this will be a precursor to #26234 — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

jreback · 2019-05-01T11:48:43Z

not exactly sure why, but this surfaced an error in our tests

pushed a patch, let's see

WillAyd · 2019-05-01T14:05:53Z

Looks like a test failure in pandas/tests/io/parser/test_textreader.py which is inadvertently expecting a return value from one of the assert functions; if you can take a look

samuelsinayoko · 2019-05-01T15:27:04Z

Will do.

…

On Wed, 1 May 2019 at 15:06, William Ayd ***@***.***> wrote: Looks like a test failure in pandas/tests/io/parser/test_textreader.py which is inadvertently expecting a return value from one of the assert functions; if you can take a look — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#25462 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AEF3BPWUZSFDCWFM75YBYH3PTGPWNANCNFSM4G2U36NA> .

-- *Sam Sinayoko* | Data Scientist *BMLL Technologies Ltd.* 10th Floor – Portland House Bressenden Place London SW1E 5RS +44(0)20 3828 9020

jreback · 2019-05-05T22:29:15Z

i pushed again, let's see

jreback · 2019-05-07T00:46:01Z

@WillAyd I think we would get a lot of mileage out of typing these assert_* functions, can you create an issue

jreback · 2019-05-07T00:56:52Z

thanks for the patch @samuelsinayoko

WillAyd · 2019-05-07T01:00:35Z

@jreback sounds good, though I was actually thinking we'd almost always ignore pandas/tests from static analysis.

In any case opened #26302 so can discuss there as needed

Remove return values for asserts

8b2f085

Unless they are context managers.

WillAyd added Testing pandas testing functions or related to the test suite Clean labels Feb 27, 2019

WillAyd added this to the 0.25.0 milestone Feb 27, 2019

datapythonista requested a review from gfyoung February 28, 2019 10:46

samuelsinayoko added 3 commits March 3, 2019 13:28

Merge branch 'master' into 25135

15510de

Dont assert functions that assert

aaca9a7

assert either raises or doesn't. If it doesn't tm.assert_* returns None, and there's no need to assert XXX is None.

flake8

220052a

samuelsinayoko added 2 commits March 3, 2019 15:03

testing.util.is_sorted raises AssertionError

394fd96

Improve is_sorted docstring

09ebf42

remove remaining return assert_* statements

60ec5ee

samuelsinayoko added 2 commits March 4, 2019 10:46

remove remaining "return assert_*" statements in testing.py

091a00e

Fix broken tests

e600bbe

gfyoung reviewed Mar 6, 2019

View reviewed changes

pandas/util/testing.py Outdated Show resolved Hide resolved

gfyoung approved these changes Mar 6, 2019

View reviewed changes

WillAyd requested changes Mar 19, 2019

View reviewed changes

pandas/util/testing.py Outdated Show resolved Hide resolved

WillAyd approved these changes Mar 19, 2019

View reviewed changes

jreback requested changes Mar 19, 2019

View reviewed changes

samuelsinayoko added 2 commits March 26, 2019 07:58

Rename is_sorted to assert_is_sorted

ce3a0a1

Merge branch 'master' of https://github.com/pandas-dev/pandas into 25135

a3370fc

samuelsinayoko added 2 commits April 7, 2019 20:18

Merge remote-tracking branch 'upstream/master' into 25135

fbd9962

Merge branch '25135' of github.com:samuelsinayoko/pandas into 25135

e2c085c

jreback requested changes Apr 7, 2019

View reviewed changes

reactivate metadata comparison

24f3b74

jreback removed this from the 0.25.0 milestone Apr 20, 2019

WillAyd mentioned this pull request Apr 29, 2019

DOC: Fix validation error type RT01 and check in CI (#25356) #26234

Merged

3 tasks

samuelsinayoko and others added 2 commits May 1, 2019 08:59

Merge remote-tracking branch 'upstream/master' into 25135

3a10a48

handle expected in integer array testing

f678030

jreback added 2 commits May 5, 2019 18:27

Merge branch 'master' into PR_TOOL_MERGE_PR_25462

1f29f3b

fixed assert

b72e99f

jreback added 2 commits May 6, 2019 20:05

Merge branch 'master' into PR_TOOL_MERGE_PR_25462

3ccbe98

fix test

f4497fd

jreback added this to the 0.25.0 milestone May 7, 2019

jreback approved these changes May 7, 2019

View reviewed changes

jreback merged commit 6df1219 into pandas-dev:master May 7, 2019

WillAyd mentioned this pull request May 7, 2019

Add Annotations to assert_*_equal functions #26302

Closed

samuelsinayoko mentioned this pull request Nov 2, 2019

26302 add typing to assert star equal funcs #29364

Closed

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove return values for asserts #25462

Remove return values for asserts #25462

samuelsinayoko commented Feb 27, 2019 •

edited

Loading

WillAyd commented Feb 27, 2019

samuelsinayoko commented Feb 27, 2019

codecov bot commented Mar 3, 2019

codecov bot commented Mar 3, 2019 •

edited

Loading

gfyoung commented Mar 4, 2019

samuelsinayoko commented Mar 4, 2019

gfyoung commented Mar 4, 2019

samuelsinayoko commented Mar 4, 2019 via email

WillAyd left a comment

jreback Mar 19, 2019

jreback Mar 19, 2019

samuelsinayoko Mar 26, 2019

samuelsinayoko Mar 26, 2019

jreback commented Mar 26, 2019

samuelsinayoko commented Mar 26, 2019 •

edited

Loading

samuelsinayoko commented Mar 26, 2019 via email

jreback commented Apr 5, 2019

samuelsinayoko commented Apr 5, 2019 via email

jreback Apr 7, 2019

jreback commented Apr 20, 2019

WillAyd commented Apr 30, 2019

samuelsinayoko commented Apr 30, 2019 via email

jreback commented May 1, 2019

WillAyd commented May 1, 2019

samuelsinayoko commented May 1, 2019 via email

jreback commented May 5, 2019

jreback commented May 7, 2019

jreback commented May 7, 2019

WillAyd commented May 7, 2019

Remove return values for asserts #25462

Remove return values for asserts #25462

Conversation

samuelsinayoko commented Feb 27, 2019 • edited Loading

WillAyd commented Feb 27, 2019

samuelsinayoko commented Feb 27, 2019

codecov bot commented Mar 3, 2019

Codecov Report

codecov bot commented Mar 3, 2019 • edited Loading

Codecov Report

gfyoung commented Mar 4, 2019

samuelsinayoko commented Mar 4, 2019

gfyoung commented Mar 4, 2019

samuelsinayoko commented Mar 4, 2019 via email

WillAyd left a comment

Choose a reason for hiding this comment

jreback Mar 19, 2019

Choose a reason for hiding this comment

jreback Mar 19, 2019

Choose a reason for hiding this comment

samuelsinayoko Mar 26, 2019

Choose a reason for hiding this comment

samuelsinayoko Mar 26, 2019

Choose a reason for hiding this comment

jreback commented Mar 26, 2019

samuelsinayoko commented Mar 26, 2019 • edited Loading

samuelsinayoko commented Mar 26, 2019 via email

jreback commented Apr 5, 2019

samuelsinayoko commented Apr 5, 2019 via email

jreback Apr 7, 2019

Choose a reason for hiding this comment

jreback commented Apr 20, 2019

WillAyd commented Apr 30, 2019

samuelsinayoko commented Apr 30, 2019 via email

jreback commented May 1, 2019

WillAyd commented May 1, 2019

samuelsinayoko commented May 1, 2019 via email

jreback commented May 5, 2019

jreback commented May 7, 2019

jreback commented May 7, 2019

WillAyd commented May 7, 2019

samuelsinayoko commented Feb 27, 2019 •

edited

Loading

codecov bot commented Mar 3, 2019 •

edited

Loading

samuelsinayoko commented Mar 26, 2019 •

edited

Loading