Skip to content

fix(ogc): read each side of a "start/end" interval string like the list form - #443

Open
bhaskargurram-ai wants to merge 1 commit into
DOI-USGS:mainfrom
bhaskargurram-ai:fix/ogc-dates-validate-interval-strings
Open

bhaskargurram-ai wants to merge 1 commit into
DOI-USGS:mainfrom
bhaskargurram-ai:fix/ogc-dates-validate-interval-strings

Conversation

@bhaskargurram-ai

Copy link
Copy Markdown
Contributor

Follow-up named in #441 ("a single string containing / is still sent unchanged, so time="garbage/.." reaches the service unvalidated").

Problem

_format_api_dates returned any single string containing / unchanged. As a result, the same range spelled the two documented ways could select different data:

input (TZ=America/Denver) sent before
time=["2024-01-01T10:00:00", None] 2024-01-01T17:00:00Z/..
time="2024-01-01T10:00:00/.." 2024-01-01T10:00:00/.. (no local-to-UTC conversion)
daily, time=["2024-01-01T10:00:00Z", "2024-02-01T00:00:00Z"] 2024-01-01/2024-02-01
daily, time="2024-01-01T10:00:00Z/2024-02-01T00:00:00Z" sent with times
time="not-a-date/also-bad" sent unchanged; the service answers 400 without naming the argument
time="2024-01-01T00:00:00Z/" sent unchanged; daily answers 400, continuous 500

Change

  • A single string containing / is split on that /, and each side is formatted exactly like an element of the two-value list form. Sides get the same parsing, naive-local-to-UTC and offset-to-Z conversion, and date truncation for date-only collections.
  • An unreadable side raises the existing ValueError (time could not be read as a date or datetime: 'not-a-date'. ...) before any request is sent.
  • An empty side is an open bound (..), and "../.." means no date filter, as [None, None] does.
  • One side may still be an ISO 8601 duration paired with an instant ("2024-01-01/P7D", "P7D/2024-01-08"), sent unchanged as before.
  • Any other shape raises ValueError naming the argument: three sides, two durations, or a duration with an open end.
  • A lone duration such as "P7D" is unchanged.
  • NEWS entry marked Behavior change.

One question for review: while testing I found that api.waterdata.usgs.gov/ogcapi/v1 rejects both duration-interval forms (daily time=2024-01-01/P7D gives 400 "Invalid datetime format", time=P7D/2024-01-08 gives 400 "Invalid ISO 8601 duration"; the same for continuous with PT2H). This PR keeps passing them through, so nothing that worked before changes. If you would rather reject them locally, it is a small change to _format_interval.

Tests

  • tests/waterdata_utils_test.py:
    • each interval string formats the same as the equivalent list (naive start/end, offset, space-separated, date-only truncation, empty end);
    • per-side formatting, including start/duration and duration/end;
    • "../.." gives None;
    • six rejected shapes, each naming the argument.
  • tests/waterdata_test.py: get_daily(time="not-a-date/also-bad") raises and sends no request.

The new cases fail on main and pass with this change. The full offline suite passes under TZ=UTC and TZ=America/Chicago, coverage is 98.99% against the 98.9 ratchet with dates.py at 100%, and ruff check, ruff format --check, mypy, lint-imports, xenon and complexipy are clean.

This touches the same module as #442, but in a different part of _format_api_dates. The only conflict between the two branches is the top entry in NEWS.md.

…st form

_format_api_dates sent any single string containing "/" unchanged. The
same range spelled as the documented string ("2024-01-01T10:00:00/..")
and as the list (["2024-01-01T10:00:00", None]) could select different
data: the string skipped the local-to-UTC conversion of naive times, the
offset-to-Z conversion, and the date truncation for date-only
collections, and "not-a-date/also-bad" reached the service unvalidated.

Split the string on its "/" and format each side as a list element is
formatted, so an unreadable side raises ValueError naming the caller's
argument before any request. An empty side becomes "..", and "../.."
means no filter, as [None, None] does. One side may still be an ISO 8601
duration paired with an instant ("2024-01-01/P7D"), kept unchanged; any
other shape (three sides, two durations, a duration with an open end)
raises ValueError naming the argument. A lone duration ("P7D") is
unchanged.

Follow-up named in DOI-USGS#441.

This branch has not been deployed

No deployments
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.

1 participant