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

Issues when str is defined as a class variable #19

Closed
max-sixty opened this issue Jul 1, 2020 · 7 comments
Closed

Issues when str is defined as a class variable #19

max-sixty opened this issue Jul 1, 2020 · 7 comments

Comments

@max-sixty
Copy link

Environment data

  • Language Server version: 2020.6.1
  • OS and version: MacOS
  • Python version (& distribution if applicable, e.g. Anaconda): Python3.7

@jakebailey asked me to open a new issue here. As discussed in #15 (comment), and microsoft/python-language-server#1732, this causes > 50 errors in https://github.com/pydata/xarray/blob/v0.15.1/xarray/core/dataarray.py.

The reproduction is there, but copying the essentials here:

It's on this line: https://github.com/pydata/xarray/blob/v0.15.1/xarray/core/dataarray.py#L242
And I suspect caused by this definition of str: https://github.com/pydata/xarray/blob/v0.15.1/xarray/core/dataarray.py#L3435

Please let me know if it would be helpful to produce logs.

Thank you!

@jakebailey
Copy link
Member

@erictraut

Maybe I'm misunderstanding the scoping rule, but given str is being declared as a class variable, is it not only accessible by say ClassName.str?

@jakebailey
Copy link
Member

I would also suggest trying to do the method described in microsoft/python-language-server#1732 (comment), though, as it would certainly prevent that aliasing.

@erictraut
Copy link
Contributor

erictraut commented Jul 1, 2020

@jakebailey, if you're within the class, you don't need to use the class name to refer to the class variable. For example, this runs fine and prints "3".

class Foo:
    str = 3
    str2 = str
    print(str2)

@jakebailey
Copy link
Member

Ah, that explains a lot. Unfortunate.

@max-sixty
Copy link
Author

This is fixed with the suggestion in (2): pydata/xarray@5c09276

It's not super convenient, as a new contributor is going to slip up at some point given mypy currently allows it, so let me reflect on which path we take. And I recognize it's on us for picking str as a class variable (I think we were following pandas convention, which was probably from a decade ago...)

Thanks for the engagement and guidance

@jakebailey
Copy link
Member

Out of curiosity, why update the docstrings to contain the "fake" type? Or are you somehow generating those docs from the type annotations?

@max-sixty
Copy link
Author

Sorry, that was a regex replace, I won't keep those

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants