geojson-pydantic icon indicating copy to clipboard operation
geojson-pydantic copied to clipboard

Cleanup Types to make Mypy and Pyright happier.

Open eseglem opened this issue 2 years ago • 3 comments

What I am changing

  • Adjusted typing throughout the package to be more complete and accurate.
  • Updated pre-commit to run mypy on the whole package vs individual files.
  • Clean up defaults so type isn't treaded as required and Optional all have default None and aren't considered required either.

How I did it

  • Tweaked mypy to use pydantic plugin and have slightly stricter rules.
  • Tweaked types until mypy no longer threw errors.
  • Tweaked types until pyright was no longer throwing errors in the module.
  • Validated typing in the tests so there were no major complaints.

Hopefully this is reasonable. It does touch a bunch of lines but I tried to avoid anything other than types.

I would have preferred to not do the if TYPE_CHECKING in types.py but the alternative was Annotated and I found a bug in pydantic that prevents nested annotated lists from working. Will keep an eye on https://github.com/samuelcolvin/pydantic/issues/4333 and could switch when it is fixed.

eseglem avatar Aug 06 '22 20:08 eseglem

@vincentsarago Updated to latest master. Should pretty much all be resolved. Left two things open for you to check but I think they are good.

eseglem avatar Aug 08 '22 13:08 eseglem

sorry @eseglem I think we can move forward with this (maybe @geospatial-jeff can also double check).

Could you please update the changelog 🙏

edit: 😂 I initially commented this in a wrong repo 😅

vincentsarago avatar Aug 31 '22 12:08 vincentsarago

Sorry for the delay, read the notification while I was away from home and slipped my mind. Updated and hopefully reasonable. @vincentsarago

eseglem avatar Sep 21 '22 17:09 eseglem

ping @geospatial-jeff for final 👍

vincentsarago avatar Dec 15 '22 10:12 vincentsarago