Skip to content

add datetime validation for collection time intervals #177

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
May 22, 2025

Conversation

vincentsarago
Copy link
Member

closes #176

Q: should we validate interval values themself? (e.g [0] < [1] and sub-intervals)

@vincentsarago vincentsarago requested a review from gadomski May 20, 2025 07:05
Copy link
Member

@gadomski gadomski left a comment

Choose a reason for hiding this comment

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

Q: should we validate interval values themself? (e.g [0] < [1] and sub-intervals)

My instinct is yes? One of the values of stac-pydantic over pure json-schema is the ability to easily do checks like this.

@vincentsarago vincentsarago requested a review from gadomski May 20, 2025 16:50
)

return v

Copy link
Member Author

Choose a reason for hiding this comment

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

moved ☝️ to shared

min_length=1,
),
AfterValidator(validate_time_interval),
]
Copy link
Member Author

Choose a reason for hiding this comment

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

@gadomski
I'm trying to understand the spec here but looking at the json-schema I feel ☝️ respect the specification

  • interval HAS TO be an array of at least 1 item [[]]
  • interval items HAVE to be an array or 2 Datetime|None value [[date, None], ...]
# previous
TimeInterval(interval=[])  # OK
TimeInterval(interval=[[]])  # OK
TimeInterval(interval=[['yo']])  # OK

# now

TimeInterval(interval=[])
ValidationError: 1 validation error for TimeInterval
interval
  List should have at least 1 item after validation, not 0 [type=too_short, input_value=[], input_type=list]
    For further information visit https://errors.pydantic.dev/2.11/v/too_short

TimeInterval(interval=[[]])
ValidationError: 1 validation error for TimeInterval
interval.0
  List should have at least 2 items after validation, not 0 [type=too_short, input_value=[], input_type=list]
    For further information visit https://errors.pydantic.dev/2.11/v/too_short

TimeInterval(interval=[['yo']])
ValidationError: 1 validation error for TimeInterval
interval.0.0
  Input should be a valid datetime or date, input is too short [type=datetime_from_date_parsing, input_value='yo', input_type=str]
    For further information visit https://errors.pydantic.dev/2.11/v/datetime_from_date_parsing

👇 https://github.com/radiantearth/stac-spec/blob/v1.0.0/collection-spec/json-schema/collection.json#extent-object
Screenshot 2025-05-20 at 9 28 48 PM

cc @alinbutu

Copy link
Member Author

Choose a reason for hiding this comment

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

I've just tried pystac validation and new behaviour seems 👍



class TimeInterval(StacBaseModel):
"""
https://github.com/radiantearth/stac-spec/blob/v1.0.0/collection-spec/collection-spec.md#temporal-extent-object
"""

interval: List[List[Union[str, None]]]
interval: Annotated[ # type: ignore
Copy link
Member Author

Choose a reason for hiding this comment

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

for some reason Mypy doesn't like conlist=

@vincentsarago vincentsarago merged commit f5c9033 into main May 22, 2025
6 checks passed
@vincentsarago vincentsarago deleted the feature/validate-time-intervals branch May 22, 2025 07:47
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.

TimeInterval not validated as per Temporal Extent Object description
2 participants