Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Preserve call arguments of generate_packet_map #170

Merged
merged 1 commit into from
Aug 8, 2023

Conversation

ItsDrike
Copy link
Member

@ItsDrike ItsDrike commented Aug 8, 2023

The generate_packet_map function is using functools.lru_cache to preserve the results, however this decorator returns a _lru_cache_wrapper type, which only preserves the return type from the original function, without preserving call arguments (or overloads).

This overrides lru_cache function during TYPE_CHECKING, and gives it a signature that preserves this information. This does however mean losing on the information that this function is cached, however for us, it's much more important to preserve call parameters and overloads so that calls to it are actually checked.

So while this is absolutely a hack and it's not ideal, it's the lesser evil when the alternative is losing on the type info from all parameters.

@ItsDrike ItsDrike added t: bug Something isn't working a: documentation Related to project's documentation (comments, docstrings, docs) p: 1 - high This should be addressed quickly a: API Related to exposed core API of the project labels Aug 8, 2023
The generate_packet_map function is using `functools.lru_cache` to
preserve the results, however this decorator returns a
`_lru_cache_wrapper` type, which only preserves the return type from the
original function, without preserving call arguments (or overloads).

This overrides `lru_cache` function during `TYPE_CHECKING`, and gives it
a signature that preserves this information. This does however mean
losing on the information that this function is cached, however for us,
it's much more important to preserve call parameters and overloads so
that calls to it are actually checked.

So while this is absolutely a hack and it's not ideal, it's the lesser
evil when the alternative is losing on the type info from all
parameters.
@ItsDrike ItsDrike force-pushed the preserve-call-args-lru-cache branch from 639d33b to 54d5fd0 Compare August 8, 2023 11:48
@ItsDrike ItsDrike merged commit 3085ff2 into main Aug 8, 2023
@ItsDrike ItsDrike deleted the preserve-call-args-lru-cache branch August 8, 2023 11:50
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
a: API Related to exposed core API of the project a: documentation Related to project's documentation (comments, docstrings, docs) p: 1 - high This should be addressed quickly t: bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant