ENH: Add typing tests according to the issue 45252 #45594

zkrolikowski-vl · 2022-01-24T17:09:16Z

Typing test stubs originally authored by Joanna Sendorek, Zbigniew Królikowski and pandas_stubs contributors: https://github.com/VirtusLab/pandas-stubs/graphs/contributors

Adapted to pandas needs by Torsten Wörtwein

Issue reference:
#45252

Summary of files and changes:

Included a number of test files under pandas/tests/typing/valid that contain functions testing different areas of pandas API. There are no assertions there - their only aim to check argument and declarations that should be allowed according to the documentation and popular use-cases.
Included a dedicated README under pandas/tests/typing/REAMDE.md that describes the purpose of origin of those files.
Included a file called STUBS_LICENSE under LICENSES. It contain the Copyright and license (MIT) of the original project.
Modified pyproject.toml to enable mypy and pyright on the directory.

Related and out-of-scope aspects

How to handle negative tests: That's out-of-scope. Simplest way would be to place those tests in a directory not checked by pytest and use # type: ignore[<reason>].
How to use these new typing test when we have more (public) pyi files TYP/CI: Demonstrate possible integration of VirtusLab's type tests #45561 (comment): Mypy or pyright should be able to automatically pick-up the .pyi files when they'll be present.
How to maintain the new type tests (ideally could de-duplicate work between pandas and VirtusLab): As for keeping up-to-date on our side I've created two issues Bring pands_stubs CI standards as close as possible to pandas VirtusLab/pandas-stubs#146 Automate pandas PR/branch creation based on snippet changes VirtusLab/pandas-stubs#147 to tackle that. At the very least we can make it easier to move the code. At best we can partially automate it. In terms of de-duplicating work - all work in pandas_stubs except for small fixes is written out in issues and it will most likely be the same for pandas. We'll need to count on contributor diligence.
closes ENH: Add set of tests to type check the public APIs #45252
tests added / passed
Ensure all linting tests pass, see here for how to run them
whatsnew entry - (doesn't have a functional impact)

Test stubs originally authored by Joanna Sendorek, Zbigniew Królikowski and pandas_stubs contributors: https://github.com/VirtusLab/pandas-stubs/graphs/contributors Adapted to pandas needs by Torsten Wörtwein

twoertwein · 2022-01-24T22:50:30Z

Except for running isort you will need to skip a few tests depending on required dependencies to make the CI greener :) (or wait first for consensus before going forward)

df.plot.hist() in pandas/tests/typing/valid/test_frame.py:534 requires matplotlib
df4: pd.DataFrame = pd.DataFrame({"a": [1], "b": [1]}).style.pipe(foo) in pandas/tests/typing/valid/test_frame.py:733 seems to require jinja2

The easiest is probably to skip the entire test function with the existing decorator:

from pandas.util import _test_decorators as td

@td.skip_if_no("matplotlib")
def test_bar():
    ...

Probably wouldn't hurt to also have a small readme in the typing/valid folder to mention where these files came from and state the main contributors. Probably would also need someone with expertise with licenses (Pandas is BSD-3 but the stubs are MIT).

zkrolikowski-vl · 2022-01-25T17:00:34Z

@twoertwein It's impossible to make both isort and black pass at the same time. They expect different order of imports. I was forced to push without the pre-commit hooks. Hopefully it works correctly on the CI.

EDIT: No it doesn't. I'll continue working on that. There might be a workaround.

I've added a small README but haven't mentioned the contributors - pandas repository seems devoid of such and I've deemed to continue with this approach.

zkrolikowski-vl · 2022-01-25T17:27:32Z

@twoertwein Only available option was to configure isort to black. This might be a bit controversial.

twoertwein · 2022-01-25T18:16:20Z

@twoertwein Only available option was to configure isort to black. This might be a bit controversial.

I think isort is already configured to be compatible with black:

pandas/pyproject.toml

Line 143 in 42a26ac

profile = "black"

pandas/tests/typing/valid/test_frame.py

twoertwein · 2022-01-25T21:14:20Z

pandas/tests/typing/README.md

+## Origins and attribution
+
+The tests come from the [pandas-stubs](https://github.com/VirtusLab/pandas-stubs)
+repository originally released under the MIT license.


@shoyer You seem to often reply to license compatibility questions :)

Is it okay to integrate this MIT-licened code into pandas?

Pandas is happy to reuse MIT licensed code (e.g., see existing examples in https://github.com/pandas-dev/pandas/tree/main/LICENSES), but you absolutely need to preserve original copyright notices. The general rule (not sure if pandas is 100% strict about it, but it's a good practice) is to keep the original copyright notice as a top level comment inside each file.

@shoyer Should I include any open source contributors, who added their code since the library was originally created, in the copyright notice?

pandas/tests/typing/valid/test_series.py

twoertwein · 2022-01-27T14:48:06Z

pandas/tests/typing/valid/test_series.py

+
+def test_types_rank() -> None:
+    s = pd.Series([1, 1, 2, 5, 6, np.nan, "million"])
+    s.rank()


This seems to work with 1.4.0 (FutureWarning) but on main this results in a TypeError
TypeError: '<' not supported between instances of 'str' and 'int'

[I have no idea why some of the tempfile.NamedTemporaryFile-tests create PermissionErrors on Windows]]

I'm leaning towards the opinion that all deprecated features shouldn't be tested - if pandas would ship with type information (which is the ultimate aim AFAIK) users should be discouraged to use them. Looks like the CI won't allow those either way.

Apparently NamedTemporaryFile cannot have two handles on Windows. Fixed that.

Would maybe be a good first invalid type test (but that is probably too much for this already very large PR) @simonjayhawkins

I've decided to remove the deprecated use-cases.

Test stubs originally authored by Joanna Sendorek, Zbigniew Królikowski and pandas_stubs contributors: https://github.com/VirtusLab/pandas-stubs/graphs/contributors Adapted to pandas needs by Torsten Wörtwein

pandas/tests/typing/valid/test_frame.py

pandas/tests/typing/valid/test_pandas.py

pandas/tests/typing/valid/test_frame.py

twoertwein

Looks good to me! Thanks @zkrolikowski-vl

There are still a few open points to discuss:

how to handle negative tests
how to use these new typing test when we have more (public) pyi files TYP/CI: Demonstrate possible integration of VirtusLab's type tests #45561 (comment)
how to maintain the new type tests (ideally could de-duplicate work between pandas and VirtusLab)

I'm not sure whether this PR needs to wait for all of the above.

github-actions · 2022-03-25T00:04:50Z

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.

simonjayhawkins · 2022-05-03T13:08:00Z

@Dr-Irv we should close this now and open these changes against https://github.com/pandas-dev/pandas-stubs in the first instance?

Dr-Irv · 2022-05-03T13:20:01Z

@Dr-Irv we should close this now and open these changes against https://github.com/pandas-dev/pandas-stubs in the first instance?

We can close it. The current pandas-stubs has an evolved version of the tests that are in this PR. No need to copy anything over.

Add typing tests according to the issue 45252.

66e4aed

Test stubs originally authored by Joanna Sendorek, Zbigniew Królikowski and pandas_stubs contributors: https://github.com/VirtusLab/pandas-stubs/graphs/contributors Adapted to pandas needs by Torsten Wörtwein

zkrolikowski-vl mentioned this pull request Jan 24, 2022

ENH: Add set of tests to type check the public APIs #45252

Open

twoertwein mentioned this pull request Jan 24, 2022

TYP/CI: Demonstrate possible integration of VirtusLab's type tests #45561

Closed

twoertwein added the Typing type annotations, mypy/pyright type checking label Jan 24, 2022

README, isort, skip_if_no

54773e5

Configured isort for black profile

5bedab9

twoertwein mentioned this pull request Jan 25, 2022

TYP: return type of read_csv/table #45610

Merged

twoertwein reviewed Jan 25, 2022

View reviewed changes

pandas/tests/typing/valid/test_frame.py Outdated Show resolved Hide resolved

twoertwein reviewed Jan 25, 2022

View reviewed changes

zkrolikowski-vl added 3 commits January 26, 2022 08:29

Configured isort for black profile

a58cd25

Copyright notice, tempfile

d2b55ab

Remove redundant profile configuration

479b6bf

twoertwein reviewed Jan 27, 2022

View reviewed changes

pandas/tests/typing/valid/test_series.py Show resolved Hide resolved

twoertwein reviewed Jan 27, 2022

View reviewed changes

pandas/tests/typing/valid/test_series.py Outdated Show resolved Hide resolved

zkrolikowski-vl added 2 commits January 27, 2022 14:41

Missing matplotlib. Isort again

feb94b9

Isort

4d1eefd

twoertwein reviewed Jan 27, 2022

View reviewed changes

zkrolikowski-vl added 9 commits January 27, 2022 16:57

Add typing tests according to the issue 45252.

fc35975

Test stubs originally authored by Joanna Sendorek, Zbigniew Królikowski and pandas_stubs contributors: https://github.com/VirtusLab/pandas-stubs/graphs/contributors Adapted to pandas needs by Torsten Wörtwein

README, isort, skip_if_no

8d17f21

Configured isort for black profile

1c32250

Configured isort for black profile

47ae975

Copyright notice, tempfile

16d21e5

Remove redundant profile configuration

e382af0

Missing matplotlib. Isort again

ff2d31b

Isort

70902cc

Windows error for tempfile

0d2b8f9

zkrolikowski-vl added 3 commits January 28, 2022 18:15

Isort

c0cc7d2

Fix

b1e0cef

Isort

16918a1

zkrolikowski-vl changed the title ~~Add typing tests according to the issue 45252~~ ENH: Add typing tests according to the issue 45252 Jan 28, 2022

Merge branch 'master' into enhancement/45252_typing_tests

98b13d4

twoertwein reviewed Jan 29, 2022

View reviewed changes

pandas/tests/typing/valid/test_frame.py Outdated Show resolved Hide resolved

zkrolikowski-vl added 5 commits February 1, 2022 19:38

Removed use of tempfile

bd002eb

Isort

6d965ee

Remove deprecated usage. Type temporary paths

2ab9bfd

Isort

4ec409f

Merge branch 'master' into enhancement/45252_typing_tests

412c1dd

twoertwein reviewed Feb 4, 2022

View reviewed changes

pandas/tests/typing/valid/test_frame.py Outdated Show resolved Hide resolved

twoertwein reviewed Feb 4, 2022

View reviewed changes

pandas/tests/typing/valid/test_frame.py Show resolved Hide resolved

twoertwein reviewed Feb 4, 2022

View reviewed changes

pandas/tests/typing/valid/test_pandas.py Show resolved Hide resolved

Fixing CI issues

8897541

twoertwein reviewed Feb 4, 2022

View reviewed changes

pandas/tests/typing/valid/test_frame.py Outdated Show resolved Hide resolved

Fixed experimental data manager issues

d1b134f

twoertwein approved these changes Feb 10, 2022

View reviewed changes

zkrolikowski-vl and others added 3 commits February 15, 2022 17:47

Merge branch 'main' into enhancement/45252_typing_tests

b1a40ea

Fixed invalid values return type for series

8f0c217

Merge branch 'main' into enhancement/45252_typing_tests

20115ab

zkrolikowski-vl marked this pull request as ready for review February 22, 2022 19:10

Merge branch 'main' into enhancement/45252_typing_tests

4c36baf

github-actions bot added the Stale label Mar 25, 2022

simonjayhawkins mentioned this pull request Apr 6, 2022

TYP: Fix core/groupby/generic.py type ignores #46521

Merged

zkrolikowski-vl closed this May 5, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ENH: Add typing tests according to the issue 45252 #45594

ENH: Add typing tests according to the issue 45252 #45594

zkrolikowski-vl commented Jan 24, 2022 •

edited

Loading

twoertwein commented Jan 24, 2022

zkrolikowski-vl commented Jan 25, 2022 •

edited

Loading

zkrolikowski-vl commented Jan 25, 2022

twoertwein commented Jan 25, 2022

twoertwein Jan 25, 2022

shoyer Jan 25, 2022

zkrolikowski-vl Jan 26, 2022

twoertwein Jan 27, 2022

zkrolikowski-vl Jan 28, 2022

twoertwein Jan 29, 2022

zkrolikowski-vl Feb 3, 2022

twoertwein left a comment

github-actions bot commented Mar 25, 2022

simonjayhawkins commented May 3, 2022

Dr-Irv commented May 3, 2022

ENH: Add typing tests according to the issue 45252 #45594

ENH: Add typing tests according to the issue 45252 #45594

Conversation

zkrolikowski-vl commented Jan 24, 2022 • edited Loading

twoertwein commented Jan 24, 2022

zkrolikowski-vl commented Jan 25, 2022 • edited Loading

zkrolikowski-vl commented Jan 25, 2022

twoertwein commented Jan 25, 2022

twoertwein Jan 25, 2022

Choose a reason for hiding this comment

shoyer Jan 25, 2022

Choose a reason for hiding this comment

zkrolikowski-vl Jan 26, 2022

Choose a reason for hiding this comment

twoertwein Jan 27, 2022

Choose a reason for hiding this comment

zkrolikowski-vl Jan 28, 2022

Choose a reason for hiding this comment

twoertwein Jan 29, 2022

Choose a reason for hiding this comment

zkrolikowski-vl Feb 3, 2022

Choose a reason for hiding this comment

twoertwein left a comment

Choose a reason for hiding this comment

github-actions bot commented Mar 25, 2022

simonjayhawkins commented May 3, 2022

Dr-Irv commented May 3, 2022

zkrolikowski-vl commented Jan 24, 2022 •

edited

Loading

zkrolikowski-vl commented Jan 25, 2022 •

edited

Loading