Pyright: Typeshedding issues with tornado and python typing

[ArktinenSieni]

Describe the bug

Pyright shows several imports of tornado and typing as "is unknown import symbol", while such classes are present in actual libraries.

Such as:

  x:x - error: "HTTPClientError" is not a known member of module (reportGeneralTypeIssues)
  x:x - error: "_GenericAlias" is unknown import symbol (reportGeneralTypeIssues)
  x:x - error: "_TypedDictMeta" is unknown import symbol (reportGeneralTypeIssues)

At least the HTTPClientError has been in use since tornado version 5.1, and project I'm working on is using 6.0.2. Besides, tornado already has types, so why does it need typeshed?

To Reproduce

1. create new virtual environemnt, activate it
   $ python3 -m venv ./.venv $ source ./.venv/bin/activate
2. install tornado to the environment
   $ python3 -m pip install tornado
3. create new python file, import items listed from above. For example:
   from typing import _GenericAlias, _TypedDictMeta from tornado.httpclient import HTTPClientError
4. run pyright to file which has imports from previous part.
   $ pyright your_file.py

Expected behavior
Should run without "unknown import symbol"-errors

VS Code extension or command-line
I'm using comand-line tool.
After brief testing using pyright VS Code-extension it makes red squiggly lines under the same imports.

Additional context
https://github.com/microsoft/pyright/issues/679#issuecomment-632882245

[shamrin]

Details about HTTPError vs HTTPClientError in Tornado:

HTTPError = HTTPClientError

https://github.com/tornadoweb/tornado/blob/v6.0.4/tornado/httpclient.py#L723

_Changed in version 5.1:_ Renamed from HTTPError to HTTPClientError to avoid collisions with tornado.web.HTTPError. The name tornado.httpclient.HTTPError remains as an alias.

https://www.tornadoweb.org/en/stable/httpclient.html#exceptions

[erictraut]

A type stub defines the public programming interface to a library. If you want to maintain compatibility with the library, you should avoid using symbols that are not part of its programming interface because these details can change as the library evolves. In this case, it sounds like these symbols have been deprecated and shouldn't be used.

If you think that there is a bug in the typeshed interface, please file a bug (or submit a PR) to the typeshed repo. Pyright simply uses the type stubs form that repo.

[shamrin]

@erictraut Makes sense about typeshed repo PR, thank you!

(Regarding deprecated symbol in Tornado, it's the other way around. Typeshed only defines the deprecated symbol and lacks the recommended symbol.)

Is there a way to tell pyright to ignore Tornado typeshed and use types from Tornado package?

[jakebailey]

I'd still report a bug on the typeshed repo for this. I would assume that they wouldn't want to have stubs in their repo if the core repo is providing the types themselves, let alone incorrect ones.

But if you mean ignore the stubs entirely and just read the real untyped code in tornado (I haven't looked to see if they provide something), then that's an odd case but could be done by deleting the stubs from the extension's directory if needed. (That is of course a short term fix.)

[shamrin]

@jakebailey, thank you for the advice! I'll submit an issue to Tornado typeshed shortly.

In the meantime, is pyright correct in its resolution order algorithm? Mypy works just fine with the examples from this issue. It seems as if mypy prefers inline types and ignores typeshed for Tornado.

I've searched for "Tornado" in typeshed and mypy issues to confirm: 1, 2, 3. It seems mypy implemented PEP 561 that says "Inline [type] packages ... [that] opts into type checking" should have precedence over typeshed. Tornado is that "inline [type] package", because it has py.typed file.

[erictraut]

Thanks for the reference to the resolution order section of PEP 561. I missed that section when I previously read that PEP.

Pyright's resolution order differs slightly from what is documented there. We'll look at aligning to the standard.

It sounds like it may no longer be necessary for typeshed to include stubs for Tornado, since it now contains inlined type information. I wonder if the typeshed maintainers would be open to simply deleting the Tornado stubs from the typeshed repo. It's worth asking when you submit your PR.

[shamrin]

Tornado stubs are being removed from typeshed now:

• https://github.com/python/typeshed/issues/4318#issuecomment-656019223
• https://github.com/python/typeshed/pull/4321

[erictraut]

That's great! I'll update the bundled typeshed stubs that ship with pyright.

[jakebailey]

One last thing we may want to consider is our import resolution mechanism; if py.typed is spec'd to be an indicator that typeshed should be ignored, we should respect that to prevent needing to change typeshed and our copy for each of these.

EDIT: Ah, Eric said something similar above, and I forgot. :)

[erictraut]

I've updated the import resolution order to match PEP 561 and updated the Pyright documentation to reflect the updated ordering. This will be in the next release.

[erictraut]

This is fixed in Pyright 1.1.52, which I just published. It will be included in the next published version of Pylance.