Skip to content

add missing type hints to __init__(...) - #2938

Merged
karpetrosyan merged 7 commits into
encode:masterfrom
pbelskiy:fix_init_types
Nov 17, 2023
Merged

add missing type hints to __init__(...)#2938
karpetrosyan merged 7 commits into
encode:masterfrom
pbelskiy:fix_init_types

Conversation

@pbelskiy

@pbelskiypbelskiy commented Nov 12, 2023

Copy link
Copy Markdown
Contributor

Hello! Thanks for awesome package! 🔥

First of all, I though about create an issue, but this is too small fix for that, dear maintainers
should I create an Issue or Discussion for that super small fixes?

Summary

Here I did little work for fixing of missed type hints of few of init()

According to https://peps.python.org/pep-0484/
(Note that the return type of __init__ ought to be annotated with -> None.

So type checkers (mypy, pyright) will work fine after this PR.

Checklist

  • I understand that this PR may be closed in case there was no previous discussion. (This doesn't apply to typos!)
  • I've added a test for each change that was introduced, and I tried as much as possible to make a single atomic change.
  • I've updated the documentation accordingly.

@karpetrosyan

Copy link
Copy Markdown
Contributor

Hi @pbelskiy! Thanks for the pull request 🙏

dear maintainers should I create an Issue or Discussion for that super small fixes?

We prefer discussion -> issue -> pull request path rather than code-change first, but I am okay reviewing this small change.

Yeah, this change makes sense, also because in some places we do not use annotation for __init__, though I found some classes where we use it.

Demo:

grep -irn __init__ tests
tests/test_multipart.py:385: def __init__(self, iterator: typing.Iterator[bytes]) -> None:
tests/test_content.py:68: def __init__(self, content: bytes) -> None:
tests/client/test_async_client.py:183: def __init__(self) -> None:
tests/client/test_async_client.py:215: def __init__(self, name: str):
tests/client/test_client.py:232: def __init__(self) -> None:
tests/client/test_client.py:263: def __init__(self, name: str):
tests/client/test_auth.py:27: def __init__(self, auth_header: str = "", status_code: int = 200) -> None:
tests/client/test_auth.py:38: def __init__(
tests/client/test_auth.py:96: def __init__(self, repeat: int):
tests/client/test_auth.py:123: def __init__(self, token: str) -> None:
tests/client/test_auth.py:142: def __init__(self) -> None:

@karpetrosyan

Copy link
Copy Markdown
Contributor

Can you also add a changelog for this pr?

@pbelskiy

Copy link
Copy Markdown
ContributorAuthor

@karpetrosyan

Okay, I will add today later, thanks!

@pbelskiy

Copy link
Copy Markdown
ContributorAuthor

@karpetrosyan

From CHANGELOG.md
All notable changes to this project will be documented in this file.

Is adding few type hints are really notable?

@karpetrosyan

Copy link
Copy Markdown
Contributor

I think we should have a changelog, even if it's a little type change, so the user having type issues with the new release can understand what could be the reason.
We do not add a changelog if it's a tool change or the user cannot interact with that change.

Other changelog that we have:

Change the type of Extensions from Mapping[Str, Any] to MutableMapping[Str, Any]. (#2803)

Anyway, if we are sure that there are no regression chances, we can skip the changelog for this change.

@pbelskiy

Copy link
Copy Markdown
ContributorAuthor

Thanks, agree with you. So I've added info to changelog.

Comment threadCHANGELOG.md Outdated
Comment threadCHANGELOG.md
@karpetrosyan
karpetrosyan merged commit 87f39f1 into encode:masterNov 17, 2023
@karpetrosyankarpetrosyan mentioned this pull request Nov 23, 2023
samclearman pushed a commit to titanmsp/httpx that referenced this pull request Apr 26, 2025
* add missing type hints to __init__
https://peps.python.org/pep-0484/
* add info to changelog
* Update CHANGELOG.md
* Update CHANGELOG.md
---------
Co-authored-by: Kar Petrosyan <92274156+karpetrosyan@users.noreply.github.com>
Co-authored-by: Tom Christie <tom@tomchristie.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@pbelskiy@karpetrosyan@lovelydinosaur