Menu

#511 Imports hidden within `if False:` prevents pyright from wroking

open
nobody
None
minor
bug
2024-04-17
2024-04-17
No

Throughout the repository, typing-related imports are hidden within an if False: block, as in the following example from ruamel.yaml.dumper

if False:  # MYPY
    from typing import Any, Dict, List, Union, Optional  # NOQA
    from ruamel.yaml.compat import StreamType, VersionType  # NOQA

Because of the conditional gate, pyright (in the latest versions) does not read the imports, and their use as annotations throughout the file is interpreted as Unknown.

The correct convention for this sort of thing is to use the TYPE_CHECKING constant imported from the typing module, which is always False at runtime but which type checkers , including mypy, interpret as True.

Naturally, TYPE_CHECKING would itself need to be imported outside of the if False: block.

While this does not affect ruamel at runtime, it does limit the utility of all the typing annotations, as pyright, one of the major type checkers, will not read them. As described above, it should be a fairly simple fix.

Discussion

  • Anthon van der Neut

    Others have asked not to import the typing module as it slows things down. So the suggested solution is not really an option. I also spent about 10x more time on mypy c.s. to stop complaining, compared to issues found, so I am unlikely to spent more time on this for a tool I don't use myself.

    I'll leave this open for the moment to see if I make this somehow env. var. dependant. At some time

     
  • Peter Van Dyken

    Peter Van Dyken - 2024-04-17

    I think it's fair not to want to spend more time on python type checking than necessary. In principle, if directly importing typing is not an option, the best solution would be to roll out .pyi files, which have 0-runtime cost. But this would involve even more work obviously.

    My own solution is just to use pyright's automatic type stub generation to make the type files I need for my own repository, and manually edit as necessary. This works well enough, especially since I use a fairly narrow segment of the API. It's just not scalable: every repository using pyright and ruamel would need to do this.

    Nevertheless, feel free to close if there's no intention of addressing, I know I don't have time to fix this properly. The ticket at least serves as a record of this limitation.

     

Log in to post a comment.