Are there plans to add type hints or library stubs for MyPy?
ginomempin opened this issue · 8 comments
I'm using your library right now in a FastAPI app, and since everything's heavily typed in FastAPI (and Pydantic), I have MyPy enabled and it's using quite a strict configuration. Right now, it's complaining about the imports from starlette_context
:
Code:
from starlette_context import context
from starlette_context.plugins import Plugin, RequestIdPlugin
Error:
Skipping analyzing 'starlette_context': found module but no type hints or library stubs [import]
Skipping analyzing 'starlette_context.plugins': found module but no type hints or library stubs [import]
See https://mypy.readthedocs.io/en/latest/running_mypy.html#missing-imports
I can always just suppress the error with # type: ignore
but I was wondering if you were planning on adding type hints or stub packages/files, as recommended by MyPy here: https://mypy.readthedocs.io/en/stable/running_mypy.html#missing-type-hints-for-third-party-library. I think that would be a better approach.
In case there's no plan for this yet, what would you say to a PR to add stub files? Basically, I think just adding the appropriate .pyi
files next to each .py
file would satisfy MyPy.
I got so used to disabling stubs in mypy.ini
that I haven't even thought about adding them.
[mypy-starlette-context.*]
ignore_missing_imports = True
It's a great idea to add them and I will be happy with your contribution.
I never looked into those things. Do you have to create stub files manually?
Stubgen does the thing. Cool!
Stubgen does the thing. Cool!
Yeah, I tried with stubgen to auto-gen the .pyi
files.
Still needs some manual updates/changes, but it's a good start.
@ginomempin for the last few days I was reading about mypy, stubgen and I was testing it in a few test apps. My conclusions:
- stubfiles are created only for versions below python3 and I plan to support only 3.7+ as that's really where asgi started
- IDE gets confused by pyi files
- my example app that uses starlette-context without pyi files hasn't had any problems with mypy as everything is static-typed anyway and that's what mypy is using by default
Can you please share your configuration? I don't think mypy should complain. Are you using mypy.ini
for configuration?
I don't use a mypy.ini or any mypy-specific configuration file. I use VS Code and with mypy enabled.
Here are some files that reproduce the error:
main.py
from starlette.applications import Starlette
from starlette_context import context
from starlette_context.middleware import ContextMiddleware
app = Starlette(debug=True)
VS Code's settings.json
"python.linting.mypyPath": "/path/to/starlette-context-test/venv/bin/mypy",
"python.linting.mypyArgs": [
"--warn-unused-configs",
"--follow-imports=normal",
"--show-error-codes"
],
VS Code's logs on running mypy on main.py
> ~/path/to/starlette-context-test/venv/bin/mypy --warn-unused-configs --follow-imports=normal --show-error-codes ~/path/to/starlette-context-test/main.py
cwd: ~/path/to/starlette-context-test
##########Linting Output - mypy##########
/path/to/starlette-context-test/main.py:4: error: Skipping analyzing 'starlette_context': found module but no type hints or library stubs [import]
/path/to/starlette-context-test/main.py:4: note: See https://mypy.readthedocs.io/en/latest/running_mypy.html#missing-imports
/path/to/starlette-context-test/main.py:5: error: Skipping analyzing 'starlette_context.middleware': found module but no type hints or library stubs [import]
Found 2 errors in 1 file (checked 1 source file)
Manually running mypy manually on main.py with default configs
$ mypy main.py
main.py:4: error: Skipping analyzing 'starlette_context': found module but no type hints or library stubs
main.py:4: note: See https://mypy.readthedocs.io/en/latest/running_mypy.html#missing-imports
main.py:5: error: Skipping analyzing 'starlette_context.middleware': found module but no type hints or library stubs
Found 2 errors in 1 file (checked 1 source file)
After re-reading MyPy's docs on Creating PEP 561 compatible packages, I realized that since everything on your package is already properly typed, it is only required that a py.typed
file is exported:
PEP 561 describes three main ways to distribute type information:
- A package has inline type annotations in the Python implementation.
......packages that supply type information via type comments or annotations in the code should put a
py.typed
file in their package directory. For example, here is a typical directory structure:setup.py package_a/ __init__.py lib.py py.typed
I retested based on my example above, and true enough, just exporting a py.typed
file as part of the package installation will resolve all the MyPy errors I was previously getting. There is no need for stub files (.pyi
) (especially if you are only targeting Python 3.7 and up).
$ ll venv/lib/python3.8/site-packages/starlette_context
total 24
-rw-r--r-- 1 gino gino 273B Apr 11 13:30 __init__.py
drwxr-xr-x 5 gino gino 160B Apr 11 13:30 __pycache__/
-rw-r--r-- 1 gino gino 1.3K Apr 11 13:30 ctx.py
-rw-r--r-- 1 gino gino 237B Apr 11 13:30 header_keys.py
drwxr-xr-x 6 gino gino 192B Apr 11 13:30 middleware/
drwxr-xr-x 11 gino gino 352B Apr 11 13:30 plugins/
-rw-r--r-- 1 gino gino 0B Apr 11 13:30 py.typed
I submitted PR #36 to basically remove all the 14 .pyi
files I added.
I am sorry for the trouble that caused. :(
It should be enough to keep the py.typed
file though.
No worries. We both learned something new :) Anyway, the way I understand it indeed only py.typed
is needed but I have mypy pre-commit hook and it's not complaining. Also, I don't have any issues using it in another projects without this file. So maybe it all boils down to the mypy config?
Anyway, it should be working well for all cases now.
released :)