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

Import difficulty with apache_beam #1734

Closed
hmc-cs-mdrissi opened this issue Apr 5, 2021 · 7 comments
Closed

Import difficulty with apache_beam #1734

hmc-cs-mdrissi opened this issue Apr 5, 2021 · 7 comments
Labels
addressed in next version Issue is fixed and will appear in next published version bug Something isn't working

Comments

@hmc-cs-mdrissi
Copy link

Describe the bug

import apache_beam as beam

test = beam.combiners.Sample()

gives an error saying Sample is not found. beam.combiners file is apache_beam/transforms/combiners.py and Sample looks defined in a normal manner.

To Reproduce
Clone this repository and follow readme. You can also just pip install latest apache beam. The repo just has a dockerfile in case the issue is environment related.

Expected behavior
No error

VS Code extension or command-line
Pyright 1.128 both directly locally on my mac and in that dockerfile that's debian based.

@erictraut
Copy link
Collaborator

Do you have the useLibraryCodeForTypes setting enabled for pyright? By default, it's disabled in pyright (but enabled in pylance). With this setting enabled, pyright attempts to extract type information from a library's source files even if the library does not claim to be "typed" (i.e. it doesn't have a "py.typed" file, as per PEP 561). The type information that pyright can extract from these untyped libraries is good enough for basic completion suggestions, but it's rarely good enough for type checking. For that reason, you should disable useLibraryCodeForTypes if you want to enable type checking. With this setting disabled, all symbols imported from untyped libraries will be treated as type Any, effectively disabling the type checker for types imported from those libraries. If you install type stubs for these libraries or newer versions of the libraries that are properly typed, you can then take advantage of the new type information.

I installed the latest version of apache_beam, and it is not a typed library (i.e. contains no type annotations, nor does it include a "py.typed" file indicating that it's typed).

@erictraut erictraut added the as designed Not a bug, working as intended label Apr 6, 2021
@hmc-cs-mdrissi
Copy link
Author

I do have useLibraryCodeForTypes enabled.

apache_beam does have some type annotations and mentions it here. The py.typed is missing though so not sure if that's a mistake or if they're intentionally not having it for some reason (maybe still in progress). I'll check beam repo and raise an issue about it if needed.

@erictraut
Copy link
Collaborator

Sounds good! Here's some guidance we've published for library authors who are looking to add type information to their packages.

@erictraut
Copy link
Collaborator

I was curious why pyright was having trouble resolving the Sample symbol from the combiners submodule. The apache_beam package is doing something that's suspect in the file apache_beam/transforms/__init__.py. It includes the statement from apache_beam.transforms import combiners. Normally from apache_beam.transforms would look in the apache_beam/transforms/__init__.py file to resolve the combiners symbol, but in this case the code is relying on the fact that this symbol is not yet defined in that module and expecting the importer to fall back on the submodule apache_beam/transfoms/combiners.py instead. In other words, it's relying on a fragile assumption about import ordering. Pyright was not handling this case in the manner in which this code intended. I've added some additional logic, and it's now able to resolve the Sample symbol when useLibraryCodeForTypes is enabled.

@jakebailey, this is one of the cases that I think you raised previously. Here's a PR for you to review: #1735

@erictraut erictraut added bug Something isn't working and removed as designed Not a bug, working as intended labels Apr 6, 2021
@layday
Copy link

layday commented Apr 6, 2021

Isn't this the same as from . import combiners, which I expect Pyright handles appropriately? Because beam supports Python 2, which does not support relative imports, it uses the full qualified name of the package.

@jakebailey
Copy link
Member

It's the same-ish, but there's an interplay when it comes to code flow and symbol tables here. It's not unlike the edge cases I described here: microsoft/pylance-release#1050 (comment)

Opening a few test apps (with some extra logging), I can only see this code getting hit in apache_beam and sqlparse.

@erictraut erictraut added the addressed in next version Issue is fixed and will appear in next published version label Apr 7, 2021
@erictraut
Copy link
Collaborator

This is addressed in pyright 1.1.129, which I just published. It will also be included in the next release of pylance.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
addressed in next version Issue is fixed and will appear in next published version bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants