Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment was marked as outdated.
This comment was marked as outdated.
This is an important dependency of geopandas which I am adding to typeshed in python/typeshed#12990. It is in the top 1000 PyPI packages https://hugovk.github.io/top-pypi-packages/
This comment has been minimized.
This comment has been minimized.
This comment was marked as resolved.
This comment was marked as resolved.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
CI now passes. |
|
This is now ready for review |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
srittau
left a comment
There was a problem hiding this comment.
Thanks, I've noticed quite a few uncommented Anys. Please note that we now require each Any to be documented with the possible types or the reason why Any is necessary. In the case of dict[str, Any] it could sometimes be better to use dict[str, Incomplete] if it might be possible to convert the dict to a TypedDict.
| import branca # type: ignore[import-not-found] # pyright: ignore[reportMissingImports] | ||
| import folium # type: ignore[import-not-found] # pyright: ignore[reportMissingImports] | ||
| import pandas as pd | ||
| import xyzservices # type: ignore[import-not-found] # pyright: ignore[reportMissingImports] | ||
| from matplotlib.colors import Colormap # type: ignore[import-not-found] |
There was a problem hiding this comment.
We usually declared a type alias for this, e.g.:
Colormap: TypeAlias = Any # from matplotlib.colorsBut I'm actually interested how well this approach works.
There was a problem hiding this comment.
It works very well in this case because these imports usage is localized to places where the optional dependency would be present at runtime, so there is no confusion for type checkers.
Thanks for the review. I replaced some |
|
Diff from mypy_primer, showing the effect of this PR on open source code: pandera (https://github.com/pandera-dev/pandera)
+ tests/geopandas/test_geopandas.py:62: error: Type application targets a non-generic function or class [misc]
+ tests/geopandas/test_geopandas.py:66: error: Type application targets a non-generic function or class [misc]
+ tests/geopandas/test_geopandas.py:110: error: Type application targets a non-generic function or class [misc]
+ tests/geopandas/test_geopandas.py:113: error: Type application targets a non-generic function or class [misc]
+ tests/geopandas/test_geopandas.py:159: error: Type application targets a non-generic function or class [misc]
+ tests/geopandas/test_geopandas.py:162: error: Type application targets a non-generic function or class [misc]
+ tests/geopandas/test_geopandas.py:289: error: Type application targets a non-generic function or class [misc]
+ tests/geopandas/test_geopandas.py:292: error: Type application targets a non-generic function or class [misc]
|
These are basically copied over from hamdanal/python-stubs. I've been using them for a while with Pylance and fixing them along the way, hopefully they are of good quality. I've also written some tests in the original repo that I did not include in the PR but might consider using some of them later if we find it necessary.