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

Add type information to LoggingContext.__call__ #79

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

sapslaj
Copy link
Contributor

@sapslaj sapslaj commented Nov 16, 2023

Adds a DecoratedCallable TypeVar and uses that in the type hints for the __call__ method in LoggingContext. This allows type information to pass through correctly when used as a decorator.

It seems like the @wraps decorator on the wrapper mangles the type info so I had to disable type checking on the return because Pyright was convinced it was wasn't the correct type, but the tests proved otherwise.

I'm not sure there's a good way to test since at runtime the function signature is getting passed through correctly. It's only on the type checking pass that things break down. It's probably fine as-is since no tests broke as a result, but figured it was worth mentioning.

Fixes #60

@sapslaj
Copy link
Contributor Author

sapslaj commented Nov 16, 2023

Looks like pre-commit failed in CI. I ran it manually on my local machine and it's passing, so looks like it's a CI issue and not something wrong with this PR.

$ pre-commit run -a
Trim Trailing Whitespace.................................................Passed
Fix End of Files.........................................................Passed
Check Yaml...............................................................Passed
Check for case conflicts.................................................Passed
Check JSON...............................................................Passed
Check for merge conflicts................................................Passed
Debug Statements (Python)................................................Passed
black....................................................................Passed
isort....................................................................Passed
flake8...................................................................Passed
bandit...................................................................Passed
pyupgrade................................................................Passed

Adds a `DecoratedCallable` TypeVar and uses that in the type hints for the
`__call__` method in `LoggingContext`. This allows type information to pass
through correctly when used as a decorator.

It seems like the `@wraps` decorator on the wrapper mangles the type info so I
had to disable type checking on the return because Pyright was convinced it was
wasn't the correct type, but the tests proved otherwise.

I'm not sure there's a good way to test since at runtime the function signature
is getting passed through correctly. It's only on the type checking pass that
things break down. It's probably fine as-is since no tests broke as a result,
but figured it was worth mentioning.

Fixes tackle-io#60
@sapslaj sapslaj force-pushed the logging-context-decorator-types branch from 6831f54 to b8d9ed1 Compare February 6, 2024 17:38
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

LoggingContext decorator isn't typechecked, which means functions it decorates become untypechecked
1 participant