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

Lack of PEP 484 type hinting #455

Open
makedin opened this issue Jan 14, 2021 · 4 comments
Open

Lack of PEP 484 type hinting #455

makedin opened this issue Jan 14, 2021 · 4 comments

Comments

@makedin
Copy link

makedin commented Jan 14, 2021

Some of the functions currently come with type hints in their docstrings, as in the following example:
https://github.com/lxc/pylxd/blob/cb59371e7f22f70f58e769cbc14793a587a832bb/pylxd/models/instance.py#L393-L396

This format is not supported by Mypy and presumably never will be.

There is now (as of Python 3.5, released in 2015) a way of embedding type hints into the code itself, as described in PEP 484. The syntax itself was introduced all the way back in PEP 3107 so there should be no compatibility problems with any Python version that hasn't been completely out of support for a long time now.

One obvious downside of going with PEP 484 type hints would be that the documentation for a given function parameter (or return value) would be split into two: the type hinting right next to its declaration and the description in the docstring. This will, of course, only be a problem when reading the code directly from the source file, and perhaps not a very big one even then. Another possible problem could be lack of PEP 484 support in some development tools, but I certainly hope that's not a real concern in 2021.

Assuming someone (e.g. myself) would be willing to do the work, are there other reasons not to switch to the modern style?

@sparkiegeek
Copy link
Contributor

Hi @makedin,

Thanks for reporting an issue in pylxd and welcome to the community.

We'd love to get a contribution to introduce type hints. If you're able to work on that, we'll review!

@makedin
Copy link
Author

makedin commented Jan 16, 2021

Hi! Glad to hear that this has a change of getting merged. I've started adding the hints, and in the process realized one more con to the proposal not mentioned in the original ticket: many of the function signatures are going to become a lot more verbose. Some of them will not fit on a single line any more.

This problem could be avoided (or at least somewhat hidden) with the use of stub files (.pyi) as described in PEP 484, but this would, in turn, mean 1) approximately doubling the number of source files and 2) moving the type definitions even further away from the actual function signature. I feel that making the hints a part of the actual code is the way to go, but I also realize that some Python programmers dislike the verbosity with passion. Just thought I'd bring this up now so that nobody needs to be surprised at review time.

@albertodonato
Copy link
Contributor

Multiline signatures are ok. We use Black for formatting, you can just run tox -e format which will format everything automatically.

@albertodonato
Copy link
Contributor

I pushed #456 which adds a tox target to run mypy checks.

With this, running tox -e check will run the typechecker.

Also, CI will automatically run it on pull requests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants