Adjust ItemProperties Validation.

[eseglem]

Fixes #130.

Rather than using python to parse / validate the values first, this lets pydantic do its thing and then ensures the fields are set properly. This means it no longer modifies the input values in place and is significantly faster.

Previously it was set as Union[dt, str] which I believe is not actually valid based on the spec. And does not match the other datetime fields in the library. As with the others, using Optional[dt] allows for the input null value, while requiring any string to be a datetime.

Before:

After:

Yes, the test_pydantic_model_validate is "slower" between the two, but that is because in the initial benchmarks it was modifying the input datetime in place and essentially cheating by already having it as a datetime for all the benchmarks.

Otherwise the test_pydantic_orjson_model_validate and test_pydantic_model_validate_json are much faster than they were before.

Benchmarks gist is here: https://gist.github.com/eseglem/ddae063eefab545c122c93a2afc6cb86

[eseglem]

And probably worth a ping to @gadomski for any thoughts as well.

[gadomski]

And probably worth a ping to @gadomski for any thoughts as well.

I apologize but I don't have the bandwidth right now for a full dive-in, but I generally am in favor of "be permissive for inputs, strict on outputs." I'm not as familiar with pydantic as either yourself or @vincentsarago, so I'll have to defer on the correct spot to transition from permissiveness-to-strictness (whether that's on serialization or deserialization).

[eseglem]

@vincentsarago Hopefully all make sense. Let me know if there are concerns. Tests are still passing for me, so should be good on that front.

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

:exclamation: No coverage uploaded for pull request base (main@c03272e). Click here to learn what that means.

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #131   +/-   ##
=======================================
  Coverage        ?   96.67%           
=======================================
  Files           ?       26           
  Lines           ?      572           
  Branches        ?        0           
=======================================
  Hits            ?      553           
  Misses          ?       19           
  Partials        ?        0           

Flag	Coverage Δ
unittests	96.67% <100.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

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

[vincentsarago]

@eseglem I'm in favour of moving forward here and for this to be published on 3.1

Can we resolve the conflicts and update the changelog?

Maybe a quick addition in the Usage section of the Readme will be a good idea as well

[vincentsarago]

friendly ping @eseglem 😄

[vincentsarago]

@thomas-maschler @gadomski can I have your 👍 before we merge this 🙏

[eseglem]

@vincentsarago Sorry I have not been responsive lately, been focused on other work. Trying to catch back up now. Is there anything you still need from me on this?

[eseglem]

Pushed two tweaks I had been messing with. Can roll them back if preferred.

I also pushed a commit moving datetime from ItemProperties to StacCommonMetadata to a separate branch: https://github.com/stac-utils/stac-pydantic/commit/f4426b43b4aa89ae56ea943ac4e543ea209d0c2c

It was not quite as simple / clean as I initially thought, so I wanted to allow for a separate conversation if necessary. Can create a PR from it after this, or add it here. Whatever works.

[vincentsarago]

@eseglem thanks 🙏

feel free to push https://github.com/stac-utils/stac-pydantic/commit/f4426b43b4aa89ae56ea943ac4e543ea209d0c2c in this branch 👍

It also appears to be missing license from here. That would be a separate issue, but I wanted to at least point it out.

feel also free to add this here

[vincentsarago]

FYI I pushed https://github.com/stac-utils/stac-pydantic/commit/f4426b43b4aa89ae56ea943ac4e543ea209d0c2c in this branch

[vincentsarago]

thanks a ton @eseglem

excellent work as usual 🙏