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

Fix type hint for func argument to inject #146

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

Conversation

dmontagu
Copy link

@dmontagu dmontagu commented Nov 8, 2024

As far as I can tell, making this change has no impact on the runtime behavior, and doesn't cause any code that currently passes type-checking to fail. However, it makes it so that doing, e.g.:

@inject(cast=False)
def my_func(my_dep: Any = Depends(my_dep_function)):
    ...

doesn't produce a type error.

@@ -48,7 +48,7 @@ def __call__(

@overload
def inject( # pragma: no cover
func: None,
func: None = None,
Copy link
Author

@dmontagu dmontagu Nov 8, 2024

Choose a reason for hiding this comment

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

I also think it would also be reasonable to just drop this argument completely from this overload:

Suggested change
func: None = None,

but with the current approach in the PR, you at least have the option to purposely use func=None as an argument if desirable for some exotic reason..

Copy link
Owner

Choose a reason for hiding this comment

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

Yes, indeed, but I think, that it should be Literal[None] = None

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.

2 participants