Skip to content

Conversation

@dimbleby
Copy link
Contributor

@dimbleby dimbleby commented Jul 4, 2022

I probably should have checked first whether this is something you even want, but if the answer is "no thanks" then fair enough: it wasn't so much work that either of us need feel bad about that.

I've added type annotations everywhere, and mypy configuration to check them. I haven't tried to update your github workflows, but all you should need to do is run like this:

$ mypy cachecontrol
Success: no issues found in 13 source files

except NameError:
text_type = str

__all__ = ["urljoin", "pickle", "HTTPResponse", "is_fp_closed", "text_type"]
Copy link
Contributor Author

@dimbleby dimbleby Jul 4, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since setup.py requires python3 https://github.com/ionrock/cachecontrol/blob/6155c199438bfc3944a833f1dec6fb74134476f1/setup.py#L27

I suppose some or more of this file could be removed

mr-c added a commit to common-workflow-language/schema_salad that referenced this pull request Sep 13, 2022
@mr-c
Copy link
Contributor

mr-c commented Sep 13, 2022

These are great! We had a very minimal set of type hints for cachecontrol; and I updated our type hints to match yours in common-workflow-language/schema_salad@be8da2e ; and our type checking still pass.

One note would be to add a mypy test in tox.ini, so that future PRs are also checked.

mr-c added a commit to common-workflow-language/schema_salad that referenced this pull request Sep 13, 2022
@mr-c
Copy link
Contributor

mr-c commented Sep 13, 2022

I would love to see this merged and in the next release :-)

This PR also adds py.typed, the PEP-561 advertisement that this package carries its own type information. This is very useful to me, thanks.

@dimbleby dimbleby requested a review from frostming May 29, 2023 12:15
@frostming frostming merged commit fe52df1 into psf:master May 30, 2023
@dimbleby dimbleby deleted the type-checking branch May 30, 2023 07:08
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

Successfully merging this pull request may close these issues.

3 participants