Skip to content

add typing information - #277

Merged
Kriechi merged 2 commits into
python-hyper:masterfrom
Kriechi:typing
Nov 23, 2024
Merged

Kriechi merged 2 commits into
python-hyper:masterfrom
Kriechi:typing

Conversation

@Kriechi

@Kriechi Kriechi commented Nov 16, 2024 •

Copy link
Copy Markdown
Member

also closes #274

@BYK BYK left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Shall we add black too in a follow up?

Comment thread src/hpack/hpack.py Outdated


def _to_bytes(string):
def _to_bytes(string: Union[bytes, str, Any]) -> bytes:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Just Any probably covers all but I like you calling out the "expected" types. Maybe add a comment about this to avoid confusion in future readers?

Comment thread src/hpack/hpack.py
indexed_name = high_byte & 0x0F
name_len = 4
not_indexable = high_byte & 0x10
not_indexable = bool(high_byte & 0x10)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Good one!

Comment thread src/hpack/huffman.py
Comment thread src/hpack/table.py


def table_entry_size(name, value):
def table_entry_size(name: Union[bytes, str], value: Union[bytes, str]) -> int:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Maybe we can abstract this Union[bytes, str] and tuple[bytes, bytes] into type definitions as they are used in a bunch of places. Something like RawHeaderPiece and UntypedHeaderTuple respectively?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yes! It was on my ToDo list, but didn't make it for this first PR - happy to be refactored in a future PR!
I'm working on a similar PR for the h2 library as well, so we might want to hold off on type aliases until we have a full picture.

Comment thread src/hpack/table.py Outdated
@Kriechi
Kriechi force-pushed the typing branch 2 times, most recently from 04b0c41 to 1534dfe Compare November 16, 2024 18:44
Comment thread src/hpack/struct.py Outdated
@Kriechi
Kriechi force-pushed the typing branch 2 times, most recently from fc455ed to 1702e21 Compare November 23, 2024 08:12
@Kriechi
Kriechi merged commit 131b44c into python-hyper:master Nov 23, 2024
@Kriechi
Kriechi deleted the typing branch November 23, 2024 08:23
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.

When will be the time to drop Python 2 compatibility?

2 participants