CLN: tests/extension/json/test_json.py typefix #28994

lukasbk · 2019-10-15T09:24:51Z

The error is due to the fact that the builtins.staticmethod type defined in extension.base.base.BaseExtensionTests does not match the function type in extension.json.test_json.BaseJSON. I don't see a possibility for a proper type hint fix without changing the structure of the code. One way would be to replace the call of staticmethod by function definitions that call the corresponding methods.
Here I propose an easier suppression of the error using type: Any.
What do you think?

xref Fix mypy errors for pandas\tests\* #28926
tests added / passed
passes black pandas
passes git diff upstream/master -u -- "*.py" | flake8 --diff
whatsnew entry

…ev#28926

…efix

The error is due to the fact that the builtins.staticmethod type defined in extension.base.base.BaseExtensionTests does not match the function type in extension.json.test_json.BaseJSON. I don't see a possibility for a type hint fix without changing the structure of the code. One way would be to replace the call of staticmethod by function definitions that call the corresponding methods. Here I propose an easier suppression of the error using type: Any. What do you think?

simonjayhawkins

@lukasbk Thanks for the PR.

This PR also has the changes from #28990, can you keep these separate.

also see #28988 re discussion on solving Definition of "assert_series_equal" in base class "BaseJSON" is incompatible with definition in base class "BaseExtensionTests" errors.

This reverts commit 217adc8.

jbrockmendel · 2019-10-17T23:54:03Z

@lukasbk can you rebase

pep8speaks · 2019-10-20T09:18:44Z

Hello @lukasbk! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2019-12-15 18:57:28 UTC

simonjayhawkins · 2019-10-21T09:08:49Z

pandas/tests/extension/base/base.py

@@ -2,8 +2,18 @@


 class BaseExtensionTests:
+    @staticmethod
+    def assert_equal(left, right, **kwargs):
+        return tm.assert_equal(left, right, **kwargs)


don't need the return

simonjayhawkins · 2019-10-21T09:08:56Z

pandas/tests/extension/base/base.py

-    assert_extension_array_equal = staticmethod(tm.assert_extension_array_equal)
+    @staticmethod
+    def assert_series_equal(left, right, **kwargs):
+        return tm.assert_series_equal(left, right, **kwargs)


simonjayhawkins · 2019-10-21T09:09:16Z

pandas/tests/extension/base/base.py

+
+    @staticmethod
+    def assert_frame_equal(left, right, **kwargs):
+        return tm.assert_frame_equal(left, right, **kwargs)


simonjayhawkins · 2019-10-21T09:09:23Z

pandas/tests/extension/base/base.py

+
+    @staticmethod
+    def assert_extension_array_equal(left, right, **kwargs):
+        return tm.assert_extension_array_equal(left, right, **kwargs)


…st_json_typefix

WillAyd · 2019-10-23T17:20:44Z

pandas/tests/extension/base/base.py

@@ -2,8 +2,18 @@


 class BaseExtensionTests:
+    @staticmethod


Can you get rid of these methods altogether and just import / use as function within tests?

I don't think so, since some tests need to override these

Hmm OK - so was this change required? Looks like using tm.assert_* in the tests now?

WillAyd · 2019-11-08T00:29:20Z

@lukasbk looks like a merge conflict; can you fix up and address comments?

simonjayhawkins · 2019-11-13T14:56:35Z

pandas/tests/extension/base/base.py

@@ -2,8 +2,18 @@


 class BaseExtensionTests:
+    @staticmethod
+    def assert_equal(left, right, **kwargs):


AFAICT it's only assert_series_equal and assert_frame_equal that currently need to be overridden in specific extension array tests, so assert_equal and assert_extension_array_equal don't need to be changed at this time.

assert_equal and assert_extension_array_equal were not needed to be overwritten. Instead they can be directly accessed from testing.

jreback · 2019-11-16T21:47:14Z

looks ok to me. I am bit concerned that we are now changing back to using tm as we do want the ability to override on these tests, but I guess that mypy will check this now.

jreback · 2019-11-20T13:13:13Z

can you rebase

WillAyd · 2019-12-13T17:38:35Z

@lukasbk can you fix up the merge conflict? I think we can get this in if so

datapythonista · 2019-12-29T14:02:14Z

@lukasbk the CI didn't pass, and this is conflicting again.

Do you mind merging master again, making sure the CI passes, and pinging us to get this merged quickly after that please? Thanks!

WillAyd · 2020-01-16T20:59:13Z

@lukasbk can you fix up merge conflict? I think this was close

simonjayhawkins · 2020-02-09T18:08:29Z

closing as stale. cc @SaturnFromTitan

lukasbk added 7 commits October 12, 2019 23:52

CLN: fix mypy errors in pandas/tests/extension/test_numpy.py pandas-d…

a19c844

…ev#28926

Applied suggestions, PR also fixes errors in test_sparse.py

28b7120

Adjusted imports

008fe48

CLN: added type hint for mypy to test_coercion.py

217adc8

Merge remote-tracking branch 'upstream/master' into test_coercion_typ…

ea903c9

…efix

reformatting

524ce17

simonjayhawkins added the Typing type annotations, mypy/pyright type checking label Oct 15, 2019

simonjayhawkins requested changes Oct 15, 2019

View reviewed changes

simonjayhawkins mentioned this pull request Oct 15, 2019

Fix mypy errors for pandas\tests\* #28926

Closed

38 tasks

lukasbk added 2 commits October 16, 2019 19:59

Revert "CLN: added type hint for mypy to test_coercion.py"

c642ade

This reverts commit 217adc8.

Merge branch 'master' into test_json_typefix

b70688f

Matching arguments of assert_..._equal methods

acf78f9

lukasbk and others added 2 commits October 20, 2019 11:27

Reformatting

6636d8e

Merge branch 'master' into test_json_typefix

88ef8dd

simonjayhawkins requested changes Oct 21, 2019

View reviewed changes

simonjayhawkins added this to the 1.0 milestone Oct 21, 2019

lukasbk added 3 commits October 23, 2019 19:10

Removed returns

cfd604c

fix setup.cfg merge conflict

d1928e9

Merge branch 'test_json_typefix' of github.com:lukasbk/pandas into te…

1549313

…st_json_typefix

WillAyd requested changes Oct 23, 2019

View reviewed changes

WillAyd mentioned this pull request Nov 8, 2019

mypy typing issues for decimal pandas/tests/extension/decimal/test_decimal.py #28988

Closed

5 tasks

simonjayhawkins reviewed Nov 13, 2019

View reviewed changes

lukasbk added 3 commits November 16, 2019 16:39

Resolve merge in setup.cfg and reduce base.py

a7fcf3f

assert_equal and assert_extension_array_equal were not needed to be overwritten. Instead they can be directly accessed from testing.

Fix stata files

d5856b8

Change more references to tm

2da8bad

Merge master

0309fe8

jreback removed this from the 1.0 milestone Jan 1, 2020

simonjayhawkins closed this Feb 9, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CLN: tests/extension/json/test_json.py typefix #28994

CLN: tests/extension/json/test_json.py typefix #28994

lukasbk commented Oct 15, 2019 •

edited

Loading

simonjayhawkins left a comment •

edited

Loading

jbrockmendel commented Oct 17, 2019

pep8speaks commented Oct 20, 2019 •

edited

Loading

simonjayhawkins Oct 21, 2019

simonjayhawkins Oct 21, 2019

simonjayhawkins Oct 21, 2019

simonjayhawkins Oct 21, 2019

WillAyd Oct 23, 2019

simonjayhawkins Nov 13, 2019

WillAyd Nov 19, 2019

WillAyd commented Nov 8, 2019

simonjayhawkins Nov 13, 2019

jreback commented Nov 16, 2019

jreback commented Nov 20, 2019

WillAyd commented Dec 13, 2019

datapythonista commented Dec 29, 2019

WillAyd commented Jan 16, 2020

simonjayhawkins commented Feb 9, 2020

CLN: tests/extension/json/test_json.py typefix #28994

CLN: tests/extension/json/test_json.py typefix #28994

Conversation

lukasbk commented Oct 15, 2019 • edited Loading

simonjayhawkins left a comment • edited Loading

Choose a reason for hiding this comment

jbrockmendel commented Oct 17, 2019

pep8speaks commented Oct 20, 2019 • edited Loading

Comment last updated at 2019-12-15 18:57:28 UTC

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

WillAyd commented Nov 8, 2019

Choose a reason for hiding this comment

jreback commented Nov 16, 2019

jreback commented Nov 20, 2019

WillAyd commented Dec 13, 2019

datapythonista commented Dec 29, 2019

WillAyd commented Jan 16, 2020

simonjayhawkins commented Feb 9, 2020

lukasbk commented Oct 15, 2019 •

edited

Loading

simonjayhawkins left a comment •

edited

Loading

pep8speaks commented Oct 20, 2019 •

edited

Loading