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

Mypy false positive: Name "asgi" already defined (by an import) #1162

Closed
peterschutt opened this issue Feb 7, 2023 · 3 comments · Fixed by #1170
Closed

Mypy false positive: Name "asgi" already defined (by an import) #1162

peterschutt opened this issue Feb 7, 2023 · 3 comments · Fixed by #1170
Labels
Bug 🐛 This is something that is not working as expected

Comments

@peterschutt
Copy link
Contributor

starlite/config/static_files.py:5: error: Name "asgi" already defined (by an import)  [no-redef]
starlite/config/static_files.py:100: error: Returning Any from function declared to return "ASGIRouteHandler"  [no-any-return]
starlite/config/static_files.py:100: error: Module not callable  [operator]

I'm getting these errors from pre-commit run mypy --all-files.

static_files.py imports asgi with from starlite.handlers import asgi, if I change that to from starlite.handlers.asgi import asgi the issue goes away.

Obviously smth to do with the fact that the name of the decorator is the same as the name of the module, but looking at starlite.handlers.__init__, the imports seem defined properly.

>>> from starlite.handlers import asgi
>>> asgi
<class 'starlite.handlers.asgi.ASGIRouteHandler'>

I think this is the relevant issue: python/mypy#7203

@peterschutt peterschutt added Bug 🐛 This is something that is not working as expected Triage Required 🏥 This requires triage labels Feb 7, 2023
@provinzkraut provinzkraut removed the Triage Required 🏥 This requires triage label Feb 7, 2023
@provinzkraut
Copy link
Member

So, I think this is more of a "questionable design choice" on our end as it's a bug. starlite.handlers.asgi is ambiguous because starlite/handlers/asgi.py as a file exists, but so does starlite.handlers.asgi, as a class.

Proper resolution IMO would be to rename starlite/handlers/asgi.py > starlite/handlers/asgi_handlers.py (and maybe the others for consistency as well?)

@peterschutt
Copy link
Contributor Author

Yep, its ambiguous as it is. I can't see why changing the module name should be that big a deal as most would be importing from the package namespace.

and maybe the others for consistency as well?

I think we'll have to for websocket too, as that should be the same issue (we mustn't expose it anywhere in the tests). While we have the chance to break stuff for 2.0 we may as well.

@Goldziher
Copy link
Contributor

Sure

Goldziher pushed a commit that referenced this issue Feb 8, 2023
* Rename handler modules to disambiguate from decorator names.

Closes #1162

* Update module refs in docs.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug 🐛 This is something that is not working as expected
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants