-
-
Notifications
You must be signed in to change notification settings - Fork 18.1k
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 Type Annotations in pandas.core.generic #25901
Comments
Also makes sense to clean up a similar error in the frame module: pandas/core/frame.py:368: error: Need type annotation for '_accessors' |
I think we need to add a comment like this:
right? I can do that. But I have never contributed to open source before so if you can help me a bit if I get stuck somewhere it will be nice. |
Hi, I tried
Should I use the the comment syntax |
Yea for local variables need comment syntax as we still support Py35 and that didn’t come until Py36.
Should use List here
…Sent from my iPhone
On Mar 28, 2019, at 4:37 AM, Vaibhav Vishal ***@***.***> wrote:
Hi, I tried _metadata: Container[str] = [] but now pep8 gives me error that:
pandas/core/generic.py:120:14: E701 multiple statements on one line (colon)
Should I use the the comment syntax # type: Container[str] or can we ignore this error?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub, or mute the thread.
|
Thanks, I will fix this and open a pull request. There are some other open issues related to Type Annotation, should I fix them too in the same PR, or you want different PR for each one? Also looking at the code |
Use List and one PR per issue for now
…Sent from my iPhone
On Mar 28, 2019, at 5:39 AM, Vaibhav Vishal ***@***.***> wrote:
Thanks, I will fix this and open a pull request. There are some other open issues related to Type Annotation, should I fix them too in the same PR, or you want different PR for each one?
Also looking at the code _metadata looks like a list to me, should I use List[str] or Container[str]
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub, or mute the thread.
|
ok. You said list in your last comment, sorry I missed that, my bad. I was looking at your very first comment which said both are containers. |
Part of #25882
Current errors are as follows:
Both of these are containers which should contain strings, so just need proper annotation and this module can be removed from mypy.ini
The text was updated successfully, but these errors were encountered: