Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix typing errors #29114

Merged
merged 1 commit into from
Oct 20, 2019
Merged

Conversation

AbhijeetKrishnan
Copy link
Contributor

@topper-123 topper-123 added Typing type annotations, mypy/pyright type checking Testing pandas testing functions or related to the test suite labels Oct 20, 2019
@topper-123 topper-123 added this to the 1.0 milestone Oct 20, 2019
@topper-123 topper-123 removed the Testing pandas testing functions or related to the test suite label Oct 20, 2019
@topper-123 topper-123 merged commit 9f03837 into pandas-dev:master Oct 20, 2019
@topper-123
Copy link
Contributor

Thanks, @AbhijeetKrishnan !

@@ -288,7 +288,10 @@ class MockFile:
assert not is_file(data)


@pytest.mark.parametrize("ll", [collections.namedtuple("Test", list("abc"))(1, 2, 3)])
test_tuple = collections.namedtuple("Test", ["a", "b", "c"])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Out of curiosity is there any issue you saw that suggested doing this? I don't think its evident why this would need to exist without knowing the context of this PR.

If it is an issue with mypy or typeshed would sometimes prefer to wait for a fix upstream; generally not in a rush with these

Copy link
Contributor Author

@AbhijeetKrishnan AbhijeetKrishnan Oct 21, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mypy does not infer that the collections.namedtuple() call returns a class of user-defined type. It, for some reason, assumes that it returns a tuple. tuple() expects an argument of Iterable[Any], which is why we see the error pandas\tests\dtypes\test_inference.py:291: error: Argument 1 to "tuple" has incompatible type "int"; expected "Iterable[Any]".

mypy also does not seem to infer that list('abc') is a literal of type List[str]. The error doesn't show up if the above one isn't corrected, but if you change ['a', 'b', 'c'] to list('abc') in the committed code, then you get the error pandas/tests/dtypes/test_inference.py:291: error: List or tuple literal expected as the second argument to namedtuple(). mypy does infer that ['a', 'b', 'c'] is of type List[str].

IMO these do seem to be limitations in mypy.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK thanks for confirming. It would be good for future edits in this project and the entire python eco-system as a whole if those were raised as issues with MyPy, if not already

Not saying this was a problem merging, but especially for internal-only annotations like this there is no rush. If something like that is raised with MyPy and looks like it would be available in a few releases would rather wait than making code edits here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@WillAyd
Copy link
Member

WillAyd commented Oct 21, 2019 via email

jbrockmendel pushed a commit to jbrockmendel/pandas that referenced this pull request Oct 23, 2019
HawkinsBA pushed a commit to HawkinsBA/pandas that referenced this pull request Oct 29, 2019
Reksbril pushed a commit to Reksbril/pandas that referenced this pull request Nov 18, 2019
proost pushed a commit to proost/pandas that referenced this pull request Dec 19, 2019
proost pushed a commit to proost/pandas that referenced this pull request Dec 19, 2019
bongolegend pushed a commit to bongolegend/pandas that referenced this pull request Jan 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Typing type annotations, mypy/pyright type checking
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants