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

Adding type hints #190

Open
wcooley opened this issue Nov 28, 2018 · 11 comments
Open

Adding type hints #190

wcooley opened this issue Nov 28, 2018 · 11 comments

Comments

@wcooley
Copy link
Contributor

wcooley commented Nov 28, 2018

I have started generating/creating typing hints for boltons and would like to make them available for others to use. There are basically 3 ways that I can do that:

  1. Add to the actual code (using Python legacy-compatible comments).
  2. Add type stubs alongside the actual code.
  3. Create a separate stub-only package (& repo).

Option 1 is the most invasive but also ensures greater consistency between the production code and hints and makes it more immediately available to someone reading the code.

Option 2 is less invasive but makes the hints less convenient for human consumption and adds a new part of the package to be maintained.

Option 3 is least invasive and requires no support from anyone other than myself but is quite likely to be neglected and poorly-maintained.

Any preference/comments?

@dmwyatt
Copy link

dmwyatt commented Dec 3, 2018

The main problem is getting other developers doing contributions in the future to add the type hints. The best solution to this is making proper type hints a pre-requisite for accepting PRs from other developers. I know of other popular projects that do this (the main one coming to mind is Home Assistant).

If you can get support from whomever approves PRs, then I fully support Option 1.

If not, I'm a little iffy on having non-typed code mixed in with type code...purely as a stylistic issue. I guess maybe I'd lean towards Option 2 in that case. Option 2 is made a little less irritating if you use something like PyCharm which should show you your types when you press Ctrl-Q or whatever you have that mapped to.

@wcooley
Copy link
Contributor Author

wcooley commented Dec 4, 2018

Thanks for the thoughts @dmwyatt.

On one hand, I agree with you about the aesthetics of mixing typed & non-typed code; on the other hand, I have to admit that pretty much all of my code is like that -- I add typing in areas where I am working, but I have generally not added it wholesale to legacy code. Even new code I sometimes realize that I've left typing out, partly because PyCharm already does a lot of type inference, so it's absence is only noticeable where that fails and it gives me bogus warnings.

In the meantime, I have created a standalone stub distribution/package:

https://github.com/wcooley/python-boltons-stubs

You'll note that there is a lot of stuff missing. Some of this is due to lack of test coverage but in some cases MonkeyType failed to either collect/store the needed info or generate the stubs from what was collected.

I am attempting to use mypy to find what is missing but I am having trouble getting it to work.

@mahmoud
Copy link
Owner

mahmoud commented Jan 17, 2019

Wow! Just how much of that repo was (pardon the pun) typed by hand? In any case, I appreciate the typeshedding. I'll weigh the options and get back to you, but at the very least I'm certainly open to documenting this more publicly for those who might be interested in this usage pattern. Thanks again!

@wcooley
Copy link
Contributor Author

wcooley commented Feb 2, 2019

Thanks @mahmoud.
The typestubs were mostly generated with MonkeyType and a fixed-up pytest-monkeytype.
There are a number of things without typestubs, because it *ahem* only generates stubs from code with test coverage.
I did have to manually generalize types, as it only generates typestubs based on the concrete types seen at runtime (which makes for interesting reading), but only for the types that I had immediate need for, so there is still much to be done.

@kasium
Copy link

kasium commented Dec 2, 2021

@mahmoud any update on this topic. I'm open to give a PR a try to add type hints 😄
@wcooley what do you think about moving your types to the typeshed repository?

@wcooley
Copy link
Contributor Author

wcooley commented Jan 7, 2022

@kasium I would be happy for them to go into either the typeshed repo or directly into boltons. I regret to say my responsibilities at work have not left me much time for working on Python, so I probably won't be able to open a PR myself any time soon.

FWIW, I don't have a license in the repo because I doubt it's copyright-able. But if necessary, it looks like I can add one through Github directly.

@kasium
Copy link

kasium commented Jan 10, 2022

Great! Let me check if I can somehow move them to typeshed... Will also take some time on my side

@wcooley
Copy link
Contributor Author

wcooley commented Jan 11, 2022

Probably the best way to do it would be to rewrite my repo and move boltons-stubs to stubs/boltons, remove the other files at the root of the directory, then make a branch of typeshed and pull the rewritten repo into it. See this for an example: https://stackoverflow.com/a/614254/627003. I guess this is probably out of date, though, since git filter-repo is now the recommended way to do this (and looks probably easier too). The really non-obvious part is that you can do git pull <path_to_repo> -- not something clearly documented. (You might also need to add --allow-unrelated-histories.)

@tuukkamustonen
Copy link

tuukkamustonen commented Jan 11, 2022

Creating and maintaining the type hints as a typeshed entry feels counter-intuitive, especially with Python 2 now gone.

Why not implement them directly in boltons code?

@guyarad
Copy link

guyarad commented Feb 26, 2023

@wcooley @kasium
best way is to add type hints into the repository, and py.typed stub to tell mypy that it's ok to use types from the source code.
See: https://mypy.readthedocs.io/en/stable/running_mypy.html#missing-imports

@Avasam
Copy link
Contributor

Avasam commented Nov 29, 2023

The main problem is getting other developers doing contributions in the future to add the type hints. The best solution to this is making proper type hints a pre-requisite for accepting PRs from other developers.

Indeed you'd want to prevent typing regressions and keep a certain baseline typing coverage. Simply running popular type-checkers (imo mypy & pyright are enough as of writing this) in the CI with as strict settings as you can support and maintain.

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

7 participants