Skip to content
New issue

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

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

Already on GitHub? Sign in 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
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.

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