asyncssh
asyncssh copied to clipboard
ronf
Pyright doesn't know exported symbols
Hello again,
while trying to code an application using your library, I noticed that pyright/pylance doesn't consider the symbols in asyncssh.* to be exported. You have a py.typed file that buys into a special behavior. Every symbol imported in the __init__.py is treated as an private import, unless you use a redundant module import. You can read about it here: https://github.com/microsoft/pyright/blob/main/docs/typed-libraries.md#library-interface the second bullet point in the second list is the relevant part to read.
It would be really nice and easier to use, if this could be fixed. If I have enough time, I will try to build a pull request for that!
Have a nice day!
Unfortunately, there are a number of incompatibilities between pyright and mypy. I tried at one point to support both, but even after getting a clean run with mypy, pyright was reporting thousands of errors, and it did not provide config options which could reasonably suppress those errors.
After spending some significant time on this, I gave up on trying to make AsyncSSH compatible with pyright. See #457 for some additional details.
I tried to run mypy now but it shows me 74 errors. Is there something specific I need to do to run mypy?
Also I just changed all exports in the asyncssh/__init__.py to from import as style and pyright is happy for now.
(At least for autocompletion). So I'm currently not sure about the incompatibilities that you mentioned. If you want to,
I can make a PR with my changes so you can decide if this is a problem or not.
Which version of mypy are you running?
With mypy 0.961 on Python 3.10, I'm seeing a small number of errors here that were masked by some local stubs I've been running with to get better type coverage for modules that were missing annotations. However, even running without those stubs, I get only 5 errors which look like they're all straightforward to correct. I'll do that.
As for pyright, changing every single one of the hundreds of symbols in __init__.py to match the "from x import y as y" syntax didn't really appeal to me, especially considering that Python itself is clearly treating these symbols as public in the "asyncssh" module without requiring that. I don't see why "pyright" shouldn't do the same.
The other option appears to be to list all the symbols in an __all__ definition in __init__.py. That's slightly less ugly to read, but it still ends up duplicating the names of all of these symbols twice in the module, which I'd really like to avoid.
I'm also concerned that pyright run on application code using AsyncSSH might produce a significant number of errors similar to those I saw when attempt to run pyright on the AsyncSSH code itself. In a quick test, the latest version of pyright (1.1.253) on Python 3.10 returned 536 errors and 1 warning despite mypy running cleanly. Some of these are potentially legitimate errors caught by pyright that were missed by mypy, but even if that's the case I'm not sure I'll have the cycles available any time soon to go through the complete list and figure that out. Also, when I last attempted this, there were many errors I couldn't "correct" without major rewrites of working code, risking breaking things in the process.
If you know of other alternative ways to fix this, I'm willing to consider them, but I really think pyright should offer a config option which makes it behave like Python itself when it comes to import visibility.
I just installed mypy from the ubuntu 22.04 package repository. It reports as version 0.942 and python is at version 3.10. I ran mypy in the root asyncssh directory with mypy . that gave me the numbers I reported above.
As for pyright: I think it just wants to improve the situation for python programmers to be able to see which functions/classes are intended to be used publicly. Look for example at my other issue. If you didn't export SSHConnectionOptions as public, this bug report might have never happened. Python on the other side obviously cannot change its defaults regarding publicity of imported symbols as this would be a major breaking change throughout the whole ecosystem. (Also python does not really have a concept of public/private in the first place, afaik) So I think this is somehow the best of both worlds. Python doesn't need to break the whole ecosystem and pyright can nudge people to be explicit about the symbols they want to publicly export. But of course that's only my point of view. I'm also mostly coming from statically compiled languages where you need to be much more explicit than in python, so for me adding a simple from ... import ... as ... is a rather small price to pay for working autocompletion.
So I don't think there are other solutions for that problem unless someone is willing to change pyright's behavior.
For reference my mypy errors:
$ mypy .
asyncssh/crypto/dh.py:30: error: Missing positional argument "q" in call to "DHParameterNumbers"
setup.py:46: error: Name "__version__" is not defined
setup.py:47: error: Name "__author__" is not defined
setup.py:48: error: Name "__author_email__" is not defined
setup.py:49: error: Name "__url__" is not defined
asyncssh/crypto/ec.py:119: error: "EllipticCurvePrivateKey" has no attribute "private_numbers"
asyncssh/crypto/ec.py:131: error: "EllipticCurvePrivateKey" has no attribute "private_numbers"
asyncssh/crypto/dsa.py:88: error: "DSAPrivateNumbers" has no attribute "private_key"
asyncssh/crypto/dsa.py:97: error: "DSAPrivateKey" has no attribute "private_numbers"
asyncssh/crypto/dsa.py:119: error: "DSAPublicNumbers" has no attribute "public_key"
asyncssh/crypto/x509.py:158: error: Argument 1 to "__init__" of "Name" has incompatible type "Iterable[RelativeDistinguishedName]"; expected "Sequence[Union[NameAttribute, RelativeDistinguishedName]]"
asyncssh/crypto/x509.py:183: error: Argument 1 to "RelativeDistinguishedName" has incompatible type "Generator[NameAttribute, None, None]"; expected "List[NameAttribute]"
asyncssh/crypto/x509.py:250: error: Argument 1 to "set" has incompatible type "ExtendedKeyUsage"; expected "Iterable[bytes]"
asyncssh/crypto/x509.py:250: note: Following member(s) of "ExtendedKeyUsage" have conflicts:
asyncssh/crypto/x509.py:250: note: Expected:
asyncssh/crypto/x509.py:250: note: def __iter__(self) -> Iterator[bytes]
asyncssh/crypto/x509.py:250: note: Got:
asyncssh/crypto/x509.py:250: note: def __iter__(self) -> Generator[ObjectIdentifier, None, None]
tests/util.py:69: error: "Type[Task[Any]]" has no attribute "all_tasks"
tests/util.py:70: error: "Type[Task[Any]]" has no attribute "current_task"
tests/util.py:75: error: Need type annotation for "_test_keys" (hint: "_test_keys: Dict[<type>, <type>] = ...")
tests/util.py:323: error: Name "ClassCleanupTestCase" already defined on line 321
tests/util.py:326: error: Need type annotation for "_class_cleanups" (hint: "_class_cleanups: List[<type>] = ...")
asyncssh/dsa.py:126: error: Need type annotation for "y"
asyncssh/sftp.py:1885: error: "stat_result" has no attribute "st_birthtime"
tests/test_known_hosts.py:50: error: Need type annotation for "keylists"
tests/test_known_hosts.py:51: error: Need type annotation for "imported_keylists"
tests/test_auth_keys.py:33: error: Need type annotation for "keylist" (hint: "keylist: List[<type>] = ...")
tests/test_auth_keys.py:34: error: Need type annotation for "imported_keylist" (hint: "imported_keylist: List[<type>] = ...")
tests/test_auth_keys.py:36: error: Need type annotation for "certlist" (hint: "certlist: List[<type>] = ...")
tests/test_auth_keys.py:37: error: Need type annotation for "imported_certlist" (hint: "imported_certlist: List[<type>] = ...")
tests/sk_stub.py:138: error: Incompatible types in assignment (expression has type "int", base class "_CtapStub" defined the type as "None")
tests/sk_stub.py:185: error: Incompatible types in assignment (expression has type "int", base class "_CtapStub" defined the type as "None")
tests/pkcs11_stub.py:183: error: Need type annotation for "tokens" (hint: "tokens: List[<type>] = ...")
tests/pkcs11_stub.py:184: error: Need type annotation for "public_keys" (hint: "public_keys: List[<type>] = ...")
tests/pkcs11_stub.py:185: error: Need type annotation for "certs" (hint: "certs: List[<type>] = ...")
docs/conf.py:182: error: Need type annotation for "latex_elements" (hint: "latex_elements: Dict[<type>, <type>] = ...")
docs/conf.py:195: error: Need type annotation for "latex_documents" (hint: "latex_documents: List[<type>] = ...")
docs/conf.py:223: error: Need type annotation for "man_pages" (hint: "man_pages: List[<type>] = ...")
docs/conf.py:235: error: Need type annotation for "texinfo_documents" (hint: "texinfo_documents: List[<type>] = ...")
tests/test_sftp.py:416: error: Need type annotation for "_ownership" (hint: "_ownership: Dict[<type>, <type>] = ...")
tests/test_public_key.py:166: error: Incompatible types in assignment (expression has type Tuple[Tuple[str, Any], Tuple[str, Any], ... <13 more items>], variable has type Tuple[Tuple[str, Any], Tuple[str, Any], ... <12 more items>])
tests/test_public_key.py:2112: error: Incompatible types in assignment (expression has type "str", base class "_TestPublicKey" defined the type as "None")
tests/test_public_key.py:2113: error: Incompatible types in assignment (expression has type "str", base class "_TestPublicKey" defined the type as "None")
tests/test_public_key.py:2114: error: Incompatible types in assignment (expression has type "Tuple[str, str, str]", base class "_TestPublicKey" defined the type as "Tuple[]")
tests/test_public_key.py:2115: error: Incompatible types in assignment (expression has type "Tuple[str, str, str, str]", base class "_TestPublicKey" defined the type as "Tuple[]")
tests/test_public_key.py:2118: error: Incompatible types in assignment (expression has type "Tuple[Tuple[str, Dict[<nothing>, <nothing>]]]", base class "_TestPublicKey" defined the type as "Tuple[]")
tests/test_public_key.py:2124: error: Incompatible types in assignment (expression has type "str", base class "_TestPublicKey" defined the type as "None")
tests/test_public_key.py:2125: error: Incompatible types in assignment (expression has type "str", base class "_TestPublicKey" defined the type as "None")
tests/test_public_key.py:2126: error: Incompatible types in assignment (expression has type "Tuple[str, str, str]", base class "_TestPublicKey" defined the type as "Tuple[]")
tests/test_public_key.py:2127: error: Incompatible types in assignment (expression has type "Tuple[str, str, str, str]", base class "_TestPublicKey" defined the type as "Tuple[]")
tests/test_public_key.py:2130: error: Incompatible types in assignment (expression has type "Tuple[Tuple[str, Dict[str, int]], Tuple[str, Dict[str, int]], Tuple[str, Dict[str, int]], Tuple[str, Dict[str, int]]]", base class "_TestPublicKey" defined the type as "Tuple[]")
tests/test_public_key.py:2139: error: Incompatible types in assignment (expression has type "str", base class "_TestPublicKey" defined the type as "None")
tests/test_public_key.py:2140: error: Incompatible types in assignment (expression has type "str", base class "_TestPublicKey" defined the type as "None")
tests/test_public_key.py:2141: error: Incompatible types in assignment (expression has type "Tuple[str, str, str]", base class "_TestPublicKey" defined the type as "Tuple[]")
tests/test_public_key.py:2142: error: Incompatible types in assignment (expression has type "Tuple[str, str, str]", base class "_TestPublicKey" defined the type as "Tuple[]")
tests/test_public_key.py:2144: error: Incompatible types in assignment (expression has type "Tuple[Tuple[str, Dict[<nothing>, <nothing>]], Tuple[str, Dict[<nothing>, <nothing>]], Tuple[str, Dict[<nothing>, <nothing>]]]", base class "_TestPublicKey" defined the type as "Tuple[]")
tests/test_public_key.py:2159: error: Incompatible types in assignment (expression has type "str", base class "_TestPublicKey" defined the type as "None")
tests/test_public_key.py:2160: error: Incompatible types in assignment (expression has type "str", base class "_TestPublicKey" defined the type as "None")
tests/test_public_key.py:2161: error: Incompatible types in assignment (expression has type "Tuple[str, str]", base class "_TestPublicKey" defined the type as "Tuple[]")
tests/test_public_key.py:2162: error: Incompatible types in assignment (expression has type "Tuple[str, str, str]", base class "_TestPublicKey" defined the type as "Tuple[]")
tests/test_public_key.py:2165: error: Incompatible types in assignment (expression has type "Tuple[Tuple[str, Dict[<nothing>, <nothing>]]]", base class "_TestPublicKey" defined the type as "Tuple[]")
tests/test_public_key.py:2175: error: Incompatible types in assignment (expression has type "str", base class "_TestPublicKey" defined the type as "None")
tests/test_public_key.py:2176: error: Incompatible types in assignment (expression has type "str", base class "_TestPublicKey" defined the type as "None")
tests/test_public_key.py:2177: error: Incompatible types in assignment (expression has type "Tuple[str, str]", base class "_TestPublicKey" defined the type as "Tuple[]")
tests/test_public_key.py:2178: error: Incompatible types in assignment (expression has type "Tuple[str, str, str]", base class "_TestPublicKey" defined the type as "Tuple[]")
tests/test_public_key.py:2181: error: Incompatible types in assignment (expression has type "Tuple[Tuple[str, Dict[<nothing>, <nothing>]]]", base class "_TestPublicKey" defined the type as "Tuple[]")
tests/test_public_key.py:2190: error: Incompatible types in assignment (expression has type "str", base class "_TestPublicKey" defined the type as "None")
tests/test_public_key.py:2191: error: Incompatible types in assignment (expression has type "str", base class "_TestPublicKey" defined the type as "None")
tests/test_public_key.py:2192: error: Incompatible types in assignment (expression has type "Tuple[str]", base class "_TestPublicKey" defined the type as "Tuple[]")
tests/test_public_key.py:2193: error: Incompatible types in assignment (expression has type "Tuple[str]", base class "_TestPublicKey" defined the type as "Tuple[]")
tests/test_public_key.py:2194: error: Incompatible types in assignment (expression has type "Tuple[Tuple[str, Dict[<nothing>, <nothing>]]]", base class "_TestPublicKey" defined the type as "Tuple[]")
tests/test_public_key.py:2216: error: Incompatible types in assignment (expression has type "str", base class "_TestPublicKey" defined the type as "None")
tests/test_public_key.py:2217: error: Incompatible types in assignment (expression has type "str", base class "_TestPublicKey" defined the type as "None")
tests/test_public_key.py:2218: error: Incompatible types in assignment (expression has type "Tuple[str]", base class "_TestPublicKey" defined the type as "Tuple[]")
tests/test_public_key.py:2219: error: Incompatible types in assignment (expression has type "Tuple[str]", base class "_TestPublicKey" defined the type as "Tuple[]")
tests/test_public_key.py:2221: error: Incompatible types in assignment (expression has type "Tuple[Tuple[str, Dict[<nothing>, <nothing>]]]", base class "_TestPublicKey" defined the type as "Tuple[]")
tests/test_pkcs11.py:84: error: Need type annotation for "_pkcs11_tokens" (hint: "_pkcs11_tokens: List[<type>] = ...")
tests/test_agent.py:95: error: Need type annotation for "_public_keys" (hint: "_public_keys: Dict[<type>, <type>] = ...")
Found 74 errors in 17 files (checked 143 source files)
Ah - only the "asyncssh" directory has type annotations so far. You should only run mypy against that, and not against the "tests" or "docs" directories, or the parent directory.
That still leaves a few errors. Some of them I just fixed yesterday. I wasn't previously seeing because I had added my own stubs in some cases where a third-party library has incorrect or missing stubs, or where I wanted to improve the type coverage. For instance, I had my own version of the "cryptography" library stubs until recently which I use here locally but didn't check in. That's no longer needed with the latest version of "cryptography", though. If you update "cryptography", do you still see the errors in asyncssh/crypto?
The one remaining error in sftp.py looks like an OS issue. The "st_birthtime" member doesn't exist on all OSes. The code is guarded, but mypy appears to not be smart enough to detect this and avoid reporting an error:
if sys.platform == 'win32': # pragma: no cover
crtime, crtime_ns = ctime, ctime_ns
elif hasattr(result, 'st_birthtime'): # pragma: no cover
crtime, crtime_ns = _float_sec_to_tuple(result.st_birthtime)
else: # pragma: no cover
crtime, crtime_ns = mtime, mtime_ns
I'm running on macOS here, so I don't see this error.
Regarding visibility of symbols, I'm not sure that changing __init__.py would have prevented the issue you saw with SSHConenctionOptions. That's actually not present in __init__.py right now -- only SSHClientConnectionOptions and SSHServerConnectionOptions are imported there. If you were referencing it, you must have been either directly importing it from one of the asyncssh subdirectories (from asyncssh/connection/connection.py) yourself or you must have been referring to it by a fully qualified name like asyncssh.connection.SSHConnectionOptions, since referencing asyncssh.SSHConnectionOptions should already be giving you an error.
Arguably, I should have made all the asyncssh subdirectories start with a leading underscore to make it more obvious that they were private and should not be directly imported from. That's the standard Python way to indicate whether a symbol is public or private and I believe mypy will give you errors or warnings if you try and use these private symbols across modules. However, by the time it occurred to me to name the directories this way, the AsyncSSH code was already public and I didn't want to make that big a disruption. I might consider doing this if I ever release a new major version.
Would you be willing to try doing this with __all__ rather than from X import Y as Y and see if that gives you what you're looking for? I think that might be more readable/maintainable if I were to do this.
Since the only symbols which should be those exposed are those at the asyncssh top-level, I think it should it be sufficient to define __all__ only in __init__.py and not any of the other subdirectories for this purpose.
I also recently ran across https://github.com/microsoft/pyright/issues/2277, where it looks like maybe pyright now has a way to control this behavior regarding private imports now. If you disable the reportPrivateImportUsage option, does it help?
I looked at the pyright issue that you mentioned and I'm pretty sure that it doesn't add any other methods to declare symbols exported. The guys are pretty explicit that as a library maintainer that put a py.typed file in its library you have no chance to specify which symbols to export as there isn't a export keyword. So the only tools we have to specify are the ones discussed earlier already.
Actually, I just tested it here, and adding the following to the top level of the pyrightconfig.json file does silence the errors, without any changes to AsyncSSH's __init__.py:
"reportPrivateImportUsage": false
From the description at microsoft/pyright#2277 it looks like that's precisely what was it created to do. Here's the relevant comment about this:
We've decided to add a new diagnostic rule. It's called "reportPrivateImportUsage". It will be on by default with basic type checking rules, but it can be disabled to retain the existing behavior. This will give you the ability to work around cases where the maintainers of a "py.typed" library haven't properly added redundant alias forms to indicate which imported symbols should be re-exported.
To avoid the need for this, I'm ok with adding __all__ as we've discussed, but the above option would be a workaround for existing versions of AsyncSSH.