Skip to content

Add type stubs - #1259

Draft
StefanBrand wants to merge 45 commits into
Toblerity:mainfrom
EOX-A:types-runtime-monkeytype
Draft

Add type stubs#1259
StefanBrand wants to merge 45 commits into
Toblerity:mainfrom
EOX-A:types-runtime-monkeytype

Conversation

@StefanBrand

Copy link
Copy Markdown
Contributor

Ref #1125

  • Initial type stubs were generated using Instagram/MonkeyType
  • Manual fixes were applied until there were no errors
  • For mypy, some dependencies are ignored in pyproject.toml
  • Pipeline was added to Github Actions

The type stubs still contain many Anys. Now we would really like to gather feedback to make the type annotations stricter, especially around the important places:

Being able to static type check writerecords() or the collection iterator would probably be the most useful thing

@StefanBrand StefanBrand mentioned this pull request May 11, 2023
Comment thread fiona/fio/helpers.pyi


def eval_feature_expression(
feature: Dict[str, Union[Dict[str, Union[str, List[List[List[float]]]]], str, Dict[str, Optional[Union[float, str, int]]]]],

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

For complex combinations of basic types, it's probably useful to introduce names for (sub)expressions.

I like the style that the httpx library uses:
https://github.com/encode/httpx/blob/master/httpx/_client.py#L1111

Comment thread fiona/rfc3339.pyi

def parse_time(
text: str
) -> Union[Tuple[int, int, int, int, int, int, int, float], Tuple[int, int, int, int, int, int, int, None], Tuple[int, int, int, int, int, int, int, int]]: ...

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This would also benefit from at least an alias.

@sgillies

Copy link
Copy Markdown
Member

@StefanBrand thank you for explaining the approach! I had not heard about MonkeyType.

Did adding type stubs find any Fiona bugs?

It will be a while before I can give my full attention to a review of this. The Fiona 1.9.4 release and some wheel building updates are higher priority for me, as are the next Rasterio release and some Shapely project governance work.

@StefanBrand

Copy link
Copy Markdown
Contributor Author

@sgillies Under my impression the type stubs derived from the tests by MonkeyType are still quite broad. We would now have to make the types stricter here and there to catch bugs if there are any. My approach would be to more correctly represent the __geo_interface__ / GeoJSON spec. We can take inspiration from https://github.com/developmentseed/geojson-pydantic, as explained in this comment: jazzband/geojson#167 (comment)

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.

5 participants