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

[suggestion] adding type hints? #199

Open
sbdchd opened this issue Oct 1, 2022 · 7 comments
Open

[suggestion] adding type hints? #199

sbdchd opened this issue Oct 1, 2022 · 7 comments

Comments

@sbdchd
Copy link
Contributor

sbdchd commented Oct 1, 2022

I see this package supports python 2.7 so the type hints would have to be the comment variety:

https://web.archive.org/web/20220213164145/https://mypy.readthedocs.io/en/stable/cheat_sheet.html

Essentially:

def add(a, b):
    # type: (int, int) -> int
    return a + b

instead of:

def add(a: int, b: int) -> int:
    return a + b

edit: another option would be some type stub files .pyi

@lopuhin
Copy link
Member

lopuhin commented Oct 3, 2022

I think dropping python 2.7 support won't be a problem

@sbdchd
Copy link
Contributor Author

sbdchd commented Oct 3, 2022

would you be open to a PR introducing type hints & dropping 2.7 support?

@lopuhin
Copy link
Member

lopuhin commented Oct 5, 2022

Yes that would be helpful 👍 If you plan to make any code changes while dropping 2.7 support (e.g. clean-up six imports and other stuff required only for python 2.7), then I think it would be better to do so in a separate PR, and then add hints.

Regarding python 3 versions, I think we can definitely drop 3.5 support. In case 3.6 support would be a problem, we can discuss dropping that as well.

@sbdchd
Copy link
Contributor Author

sbdchd commented Oct 6, 2022

Also, any thoughts on adding something like black for auto formatting? makes writing the types a little less cumbersome from the formatting changes

@lopuhin
Copy link
Member

lopuhin commented Oct 7, 2022

Re black, we use it in a few projects, such as https://github.com/scrapinghub/web-poet/ and https://github.com/scrapinghub/scrapy-poet, and the folks running these projects are happy with the setup (cc @BurnzZ ). Do you have a preferred approach or experience migrating other projects @sbdchd ?

@BurnzZ
Copy link
Member

BurnzZ commented Oct 7, 2022

It'd be cool if we also have pre-commit hooks available with isort and flake8 as well. :) Ref: https://github.com/scrapinghub/web-poet/blob/master/.pre-commit-config.yaml

@sbdchd
Copy link
Contributor Author

sbdchd commented Oct 8, 2022

yeah basically in my experience adding type hints changes a ton of formatting, so having black helps a bit -- plus I like the look of the output :D

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