Improve type hints #206
Improve type hints #206
Conversation
| if sys.version_info >= (3, 12): | ||
| fp.write("\n\nfrom collections.abc import Buffer\n\n") | ||
| else: | ||
| fp.write("\n\nBuffer = typing.Union[bytes, bytearray, memoryview]\n\n") |
There was a problem hiding this comment.
Shouldn't this use typing_extensions per the PEP? https://peps.python.org/pep-0688/#equivalent-for-older-python-versions
There was a problem hiding this comment.
Yeah but that would be an extra dependency. Pure Commands will depend on typing_extensions. Idk if every robotpy-build project should too.
There was a problem hiding this comment.
Wrap it in a try except ImportError then?
| if sys.version_info >= (3, 12): | ||
| fp.write("\n\nfrom collections.abc import Buffer\n\n") | ||
| else: | ||
| fp.write("\n\nBuffer = typing.Union[bytes, bytearray, memoryview]\n\n") |
There was a problem hiding this comment.
Version conditions for imports should go into the generated type stub, not here. Otherwise asking type checkers to target older versions will break.
There was a problem hiding this comment.
I did it that way since the wheels are version specific anyway. I'll put them in the stubs so it's the same everywhere.
| if sys.version_info >= (3, 12): | ||
| fp.write("\n\nfrom collections.abc import Buffer\n\n") | ||
| else: | ||
| fp.write("\n\nBuffer = typing.Union[bytes, bytearray, memoryview]\n\n") |
There was a problem hiding this comment.
This seems like something that should be contributed upstream, rather than us monkeypatching it in.
There was a problem hiding this comment.
It is an existing issue: sizmailov/pybind11-stubgen#87
But right now, I don't actually check if the type hints use buffer or if the type hint will override another custom type called buffer. That bit gets injected into every file. I would want to make sure the buffer stuff is only added when necessary+valid before upstreaming.
|
|
||
| def write(self, s: str) -> None: | ||
| # fix underscored names | ||
| s = re.sub(r"([a-zA-Z0-9_]+)\._\1", r"\1", s) |
There was a problem hiding this comment.
When wpi* depends on wpiutil for example, import wpiutil._wpiutil is added to the top of the stubs. This changes that to import wpiutil. I haven't tested this yet but I'm hoping that it tricks intellisense into providing better imports.
Right now, when my students write code and let vscode auto import stuff, it always goes all the way to the underscored packages.
There was a problem hiding this comment.
This feels like a really blunt instrument for doing that.
|
I vote to close this in favour of #211. |
|
Obsolete by #211. |
wpiutil._wpiutil.xwithwpiutil.x