Correctly set default values, fix start/end/datetime field validators #135
Conversation
…c-130-validate-item-properties
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #135 +/- ##
=======================================
Coverage ? 96.90%
=======================================
Files ? 26
Lines ? 613
Branches ? 0
=======================================
Hits ? 594
Misses ? 19
Partials ? 0
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
|
|
||
| def datetime_to_str(d: dt) -> str: | ||
| if d.tzinfo is None: | ||
| d = d.replace(tzinfo=timezone.utc) | ||
|
|
||
| timestamp = d.isoformat(timespec="auto") | ||
| zulu = "+00:00" | ||
| if timestamp.endswith(zulu): | ||
| timestamp = f"{timestamp[: -len(zulu)]}Z" | ||
|
|
||
| return timestamp | ||
|
|
||
|
|
||
| # Allows for some additional flexibility in the input datetime format. | ||
| # If the input value has timezone information, it will be converted to UTC timezone. | ||
| # Otherwise URT timezone will be assumed. | ||
| UtcDatetime = Annotated[ | ||
| Union[str, dt], | ||
| # Input value must be in a format which has timezone information | ||
| AfterValidator(lambda d: d if isinstance(d, dt) else dateutil.parser.isoparse(d)), | ||
| # Use `isoformat` to serialize the value in an RFC3339 compatible format | ||
| PlainSerializer(lambda d: datetime_to_str(d)), | ||
| ] |
There was a problem hiding this comment.
This might have to go depending on outcome of #131
| @pytest.mark.parametrize( | ||
| "datetime", ["2022-01-01T00:00:00", "2022-01-01", "2022-01", "2022"] | ||
| ) | ||
| def test_item_datetime_no_tz(datetime): | ||
|
|
||
| item_data = { | ||
| "type": "Feature", | ||
| "id": "sample-item", | ||
| "stac_version": "1.0.0", | ||
| "geometry": {"type": "Point", "coordinates": [125.6, 10.1]}, | ||
| "bbox": [125.6, 10.1, 125.6, 10.1], | ||
| "properties": {"datetime": datetime}, | ||
| } | ||
| item = Item(**item_data) | ||
|
|
||
| item_json = item.model_dump(mode="json") | ||
|
|
||
| # The model should fix the date and timezone for us | ||
| assert item_json["properties"]["datetime"] == "2022-01-01T00:00:00Z" |
There was a problem hiding this comment.
This might have to go depending on outcome of #131
| # def test_invalid_temporal_search_date(): | ||
| # # Just a date, no time | ||
| # utcnow = datetime.now(timezone.utc).strftime("%Y-%m-%d") | ||
| # with pytest.raises(ValidationError): | ||
| # Search(collections=["collection1"], datetime=utcnow) | ||
|
|
There was a problem hiding this comment.
This might have stay depending on the outcome of #131
|
This PR has changes that have been merged in main in small PR. For the easing the review @thomas-maschler what do you think about closing this PR, wait for #131 to be merged and then open a new PR with the proposed changes? |
|
Yep, that was the plan and why I kept it as draft |
| # If there is only one date, insert a None for the start date | ||
| if len(values) == 1: | ||
| values.insert(0, None) |
There was a problem hiding this comment.
I ran into this MR after I created #145. I see that the above matches the current behavior, nice.
But I also think the start and end should be the same if a single value is given. (Otherwise one cannot tell the difference between a single value, and a open-ended range, without looking at the original datetime again.) So I guess this could be changed to:
# If there is only one date, duplicate to use for both start and end dates
if len(values) == 1:
values.insert(0, values[0])
Or maybe easier to read then:
values.append(values[0])
|
@thomas-maschler it might be easier to start a new PR than try to update this one 🙏 |
|
Yes, I realized this. We are in the middle of a data release. I will be able to work on this again in early June. |
typeproperty to be set for Catalog and Collectionsdatetimeand Common MetaDatastart_datetimeandend_datetimedatetimeandlicenseto Common MetaDataKeeping this as DRAFT until #131 is merged