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

ENH: Using GeoDataFrame constructor fails type-checking when including "geometry" or "crs" arguments #3115

Open
dluks opened this issue Dec 29, 2023 · 2 comments

Comments

@dluks
Copy link

dluks commented Dec 29, 2023

Is your feature request related to a problem?

I use Pylance for type checking, and when I create a geopandas.geodataframe.GeoDataFrame with either geometry or crs as keyword arguments it returns a type error (see code below).

Describe the solution you'd like

Ideally, simple GeoDataFrame constructor usage wouldn't trip such general type errors.

API breaking implications

I'm not really sure. I know there are various issues in this neighborhood (e.g. #2280, #2133, #3091).

Describe alternatives you've considered

As mentioned, this seems likely to be a side-effect (or outstanding item) of some of the work that's been done to the GeoDataFrame constructor already, but I'm not familiar enough with the source code to see why. Perhaps this is one of the issues #3091 is hoping to address?

Example code

import geopandas as gpd
import numpy as np
import pandas as pd


df = pd.DataFrame(
    {
        "x": np.random.randint(-180, 180, 1000),
        "y": np.random.randint(-90, 90, 1000),
        "variable": np.random.random(1000),
    }
)

gdf = gpd.GeoDataFrame(
    df.variable, geometry=gpd.points_from_xy(df.x, df.y), crs="EPSG:4326"
)

Pylance type checking returns: No overloads for "__new__" match the provided arguments Argument types: (Series, GeometryArray, Literal['EPSG:4326']) if reportGeneralTypeIssues is enabled.

To be clear, the above code works, but for those who use type checkers it can be annoying.

@martinfleis
Copy link
Member

Geopandas does not yet support type hints and type checking. It is on the roadmap and some parts of the codebase already contain type hints but most of it does not. Issues like this are expected at this point.

@m-richards
Copy link
Member

m-richards commented Dec 30, 2023

(Disclaimer, I'm not a regular pylance user, so this might not be 100% correct) Having a quick look, this seems a bit tricky to solve within geopandas. Looking at the issue, it comes from pyright, which is bundled as part of pylance.

The __new__ method refered to is

class DataFrame(NDFrame, OpsMixin):
    __hash__: ClassVar[None]  # type: ignore[assignment]

    @overload
    def __new__(
        cls,
        data: ListLikeU
        | DataFrame
        | dict[Any, Any]
        | Iterable[ListLikeU | tuple[Hashable, ListLikeU] | dict[Any, Any]]
        | None = ...,
        index: Axes | None = ...,
        columns: Axes | None = ...,
        dtype=...,
        copy: _bool = ...,
    ) -> Self: ...
    @overload

which is part of https://github.com/pandas-dev/pandas-stubs, which is also bundled as part of pylance. Now, I don't quite understand why the stub defines an overload for __new__ and not __init__ as neither pd.DataFrame nor the parent class NDFrame define a __new__ (and nor does GeoDataFrame for that matter) (Edit: looks like that was done as part of microsoft/python-type-stubs#183, apparently to appease mypy).

Our current intent is to type geopandas inline with type annotations, rather than providing separate .pyi stub files - and as such we would never overload this annotation as we'd be annotating __init__ and not __new__ .

The premise of pyright / pylance presuming that the parent class stubs should apply to child class constructors seems incorrect, as by definition the type of the class is known when it is constructed, so there's no need to obey liskov substitution rules. Though type checking is quite complicated, and I'm not across all those complexities, so there may well be a good reason this behaves the way it does.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants