poliastro / czml3

Python 3 library to write CZML
https://pypi.org/project/czml3/
MIT License
42 stars 33 forks source link

attr -> pydantic #154

Closed Stoops-ML closed 6 days ago

Stoops-ML commented 1 week ago

Replace attrs with pydantic.

@astrojuanlu Let me know your thoughts.

codecov[bot] commented 1 week ago

Codecov Report

Attention: Patch coverage is 99.30556% with 5 lines in your changes missing coverage. Please review.

Project coverage is 99.44%. Comparing base (56632e3) to head (1b4ead7). Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
src/czml3/types.py 98.16% 4 Missing :warning:
src/czml3/enums.py 96.42% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #154 +/- ## ========================================== + Coverage 99.28% 99.44% +0.16% ========================================== Files 12 11 -1 Lines 837 898 +61 ========================================== + Hits 831 893 +62 + Misses 6 5 -1 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

astrojuanlu commented 1 week ago

Oh I love this idea! I think it makes a ton of sense. Cannot look into the code just yet but ping me again when the CI is green

Stoops-ML commented 1 week ago

What's your decision on typing? If we're using Self and StrEnum then we're limited to Python 3.11+ (see my original post).

astrojuanlu commented 1 week ago

If we're using Self and StrEnum then we're limited to Python 3.11+

Aren't there backports in typing_extensions or similar?

Stoops-ML commented 1 week ago

Python 3.7 support has been dropped as Pydantic>=2.10.0 doesn't support it.

@astrojuanlu

astrojuanlu commented 1 week ago

Good for me, 3.7 is EOL anyway. So is 3.8, by the way. Let's drop both.

I see you replaced | with Union everywhere, is that necessary?

Stoops-ML commented 1 week ago

Python 3.8 dropped.

pydantic doesn't work with | on Python 3.9 even with from __future__ import annotations specified. Someone noted the problem on SO here.

Want to drop Python 3.9 or implement Union?

astrojuanlu commented 1 week ago

Hm interesting. Well, let's drop Python 3.9 then!

Stoops-ML commented 1 week ago

All checks passed!