jazzband / geojson

Python bindings and utilities for GeoJSON
https://pypi.python.org/pypi/geojson/
BSD 3-Clause "New" or "Revised" License
898 stars 120 forks source link

Feature request: Static type support using TypedDict #167

Open kylebarron opened 2 years ago

kylebarron commented 2 years ago

👋

Recent versions of Python support optional static typing, including TypedDict, a way to define the types of properties on a dictionary. I see this project already requires 3.6 or higher, and a backport of TypedDict is in typing-extensions for Python 3.6 and Python 3.7.

I see all classes in this package currently subclass from dict: https://github.com/jazzband/geojson/blob/0041e1d868ec51571800d003888b331c0e488f3f/geojson/base.py#L5

This means it wouldn't be hard to add typing support, by removing the dict superclass and replacing it with typing.TypedDict (or typing_extensions.TypedDict for 3.6 and 3.7).

I'd be happy to submit a PR, but figured I should make an issue first to discuss.

rayrrr commented 2 years ago

Hi @kylebarron good suggestion. Given that the objects defined in the GeoJSON spec (RFC7946) are explicitly typed themselves, TypedDict could fit in here.

Please submit a PR if you still can; be sure to include per-key type specifications for all dict data structures currently used by classes representing GeoJSON objects in this package.

Thanks!

trcmohitmandokhot commented 1 year ago

I would like to help with this, and maybe as a first step watch/help advance someone else's source code if they've got it going. if no-one else has a draft PR that has this going, but can point me to a few references sources of implemetation, that would be awesome. I can read and attempt to model them. Hoping to ask one clarification question though... Is it desired via this enhancement that if someone tries to initialize an GeoJSON object in an invalid format, their type-checker gives them an prompt identifying the expected parameter type? An example of incorrect initialization below:

from geojson import Point
point_a = Point([-115.81, 37.24])

The above attempt to initialize via a List is caught by a type-checker, and a prompt shows up to say coords: tuple. The same behavior is to be achieved for other GeoJSON classes. Is this what the end-goal is for this enhancement?

trcmohitmandokhot commented 1 year ago

@rayrrr or other maintainers - Is this feature enhancement still of interest? If yes, I can work on it. Please advise! Thanks for your time.

kylebarron commented 1 year ago

I never started work on this. I briefly looked into it and felt that there were issues with typed dict that would make it hard... but in Python 3.11 NotRequired was added to typing and this functionality was backported into typing_extensions for pre-3.11. So you can now accurately represent that i.e. a bbox is an optional member of Feature.

class Feature(TypedDict):
   properties: dict
   bbox: NotRequired[Tuple[float, ...]]

I'd also suggest taking a look at https://github.com/developmentseed/geojson-pydantic as a reference. For example, in https://github.com/developmentseed/geojson-pydantic/blob/b20bd7ed7c3475d6df650430c864259bb246fcb0/geojson_pydantic/features.py they've implemented generics on the Feature class. So for example you can constrain the type of your properties by setting

class MyPropertiesLayout(TypedDict):
    property1: str

my_feature_type = Feature[Point, MyOwnTypedDict]
trcmohitmandokhot commented 1 year ago

Just a quick update. I have started to work on this feature enhancement. One of my first steps is to document the members needed for each GeoJSON object per the RFC. I will consolidate this information in the next few days. I am also looking at how to include type hinting specially in nested classes. This is where kylebarron's suggestion is likely to help. Thank you.

fmotaf commented 9 months ago

Is this issue just waiting for someone to review https://github.com/Toblerity/Fiona/pull/1259 or is it still open? I want to contribute but in doubt if it's already done and waiting for review.