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 geopandas stubs #12990

Merged
merged 11 commits into from
Jan 19, 2025
Merged

Add geopandas stubs #12990

merged 11 commits into from
Jan 19, 2025

Conversation

hamdanal
Copy link
Contributor

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.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

@hamdanal

This comment was marked as outdated.

hamdanal added a commit to hamdanal/stub_uploader that referenced this pull request Nov 10, 2024
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/
@hamdanal hamdanal closed this Nov 11, 2024
@hamdanal hamdanal reopened this Nov 11, 2024

This comment has been minimized.

@hamdanal

This comment was marked as resolved.

This comment has been minimized.

@srittau srittau closed this Nov 25, 2024
@srittau srittau reopened this Nov 25, 2024

This comment has been minimized.

This comment has been minimized.

@srittau
Copy link
Collaborator

srittau commented Nov 25, 2024

CI now passes.

@hamdanal hamdanal marked this pull request as ready for review November 25, 2024 20:02
@hamdanal
Copy link
Contributor Author

This is now ready for review

This comment has been minimized.

This comment has been minimized.

Copy link
Collaborator

@srittau srittau left a comment

Choose a reason for hiding this comment

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

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.

Comment on lines +4 to +8
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]
Copy link
Collaborator

Choose a reason for hiding this comment

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

We usually declared a type alias for this, e.g.:

Colormap: TypeAlias = Any  # from matplotlib.colors

But I'm actually interested how well this approach works.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

@hamdanal
Copy link
Contributor Author

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.

Thanks for the review. I replaced some Anys by Incompletes and commented on all the remaining ones.

Copy link
Contributor

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]

@srittau srittau merged commit c01e731 into python:main Jan 19, 2025
63 checks passed
@hamdanal hamdanal deleted the geopandas branch January 19, 2025 18:44
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