Skip to content

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

Merged
thodson-usgs merged 3 commits into
DOI-USGS:mainfrom
bhaskargurram-ai:fix/ogc-dates-validate-interval-strings
Oct 12, 2026
Merged

thodson-usgs merged 3 commits into
DOI-USGS:mainfrom
bhaskargurram-ai:fix/ogc-dates-validate-interval-strings

Conversation

@bhaskargurram-ai

@bhaskargurram-ai bhaskargurram-ai commented Oct 9, 2026 •

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.


Maintainer update (2026-10-12)

The branch is now three commits: the original commit above, unchanged; a merge of main; and one commit with the maintainer's edits. Those edits change several points above.

  • Duration sides are rejected. The services reject both duration-interval forms with HTTP 400, as the question above found, so "2024-01-01/P7D" and "P7D/2024-01-08" now raise ValueError naming the argument. A lone duration ("P7D") is unchanged.
  • The open-start form depends on the API. The NGWMN and STAC firewalls reject any ../ with HTTP 403, so get_water_level(datetime=[None, end]) and get_ratings(time=[None, end]) always failed. They now send an empty start (/end), which both services treat as open. Water Data keeps .., because it rejects /end with HTTP 400. The form is set by OgcDialect.open_start.
  • Naive dates are UTC, as in pandas. A naive date or time is UTC instead of the machine's local time (get_nearest_continuous already did this), and an aware value is converted to UTC, including in date-only collections. With local time, parsing the string like the list form would have made the documented time="2024-03-01/2024-03-02" start at 06:00Z in Chicago. Checked live on a machine set to Chicago time: it now sends 2024-03-01T00:00:00Z/2024-03-02T00:00:00Z.
  • Fractional seconds are kept instead of truncated, because truncating one changed a closed bound. The Water Data, NGWMN and STAC services all accept them (checked 2026-10-12).
  • More inputs are accepted. A minute-precision time (2024-01-01T10:00Z) and spaces around the / are accepted again; both were previously sent unchanged and worked.
  • Docs and NEWS.

A new test checks the UTC rule under TZ=America/Chicago; it fails on the old code. The full offline suite (1313 tests) passes under TZ=UTC, TZ=America/Chicago and TZ=Asia/Kolkata, and so do ruff, mypy, the pre-commit gates and the related live tests.

…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.
thodson-usgs added a commit to bhaskargurram-ai/dataretrieval-python that referenced this pull request Oct 11, 2026
The NGWMN and STAC firewalls answer any datetime containing '../' with
HTTP 403, so get_water_level(datetime=[None, end]) and
get_ratings(time=[None, end]) never worked; and since DOI-USGS#443 formats a
'/end' string, the empty-start workaround stopped working too. The
Water Data API is the reverse: it accepts '../end' and answers '/end'
with HTTP 400.

OgcDialect.open_start records the spelling (default '..'); NGWMN sets
'', and get_ratings passes '' to _format_api_dates directly since STAC
has no dialect. An open end stays '..', which all three accept.
@thodson-usgs
thodson-usgs force-pushed the fix/ogc-dates-validate-interval-strings branch from f6cbbbb to 737613b Compare October 12, 2026 17:03
@thodson-usgs
thodson-usgs force-pushed the fix/ogc-dates-validate-interval-strings branch from 737613b to 78e81eb Compare October 12, 2026 17:14
@thodson-usgs
thodson-usgs force-pushed the fix/ogc-dates-validate-interval-strings branch from 78e81eb to 7351697 Compare October 12, 2026 18:52
- Reject a duration as either side of an interval string. The Water Data
  and NGWMN services reject '2024-01-01/P7D' and 'P7D/2024-01-08' with
  HTTP 400, and that error does not name the argument. _split_interval
  raises instead and passes the two sides to the list path as a tuple,
  which removes the duration handling in _format_interval and the
  _is_passthrough helper.
- Write an open range start in the form each API accepts. The NGWMN and
  STAC firewalls reject any datetime containing '../' with HTTP 403, so
  get_water_level(datetime=[None, end]) and get_ratings(time=[None, end])
  always failed. The Water Data API accepts '../end' and rejects '/end'
  with HTTP 400. OgcDialect.open_start holds the form (default '..');
  NGWMN sets '', and get_ratings passes '' to _format_api_dates because
  STAC has no dialect.
- Keep fractional seconds instead of truncating them, which changed a
  closed bound. The Water Data, NGWMN, and STAC services all accept them
  (checked 2026-10-12). Accept a minute-precision time and spaces around
  the '/'; both were previously sent unchanged and worked.
- Read a naive date or time as UTC, as pandas does and as
  get_nearest_continuous already did, instead of as the machine's local
  time. Convert an aware value to UTC, including in a date-only
  collection. Now that the string form is parsed like the list form,
  local time would have made time="2024-03-01/2024-03-02", the example
  in the docs, start at 06:00Z in Chicago and on the previous day in
  Sydney.
- Docstrings: get_ratings no longer says time is passed through
  unchanged, and the _format_api_dates notes describe open_start. The
  duration test ids name the side that is a duration.
- Date the NEWS entry by the day it lands and place it above DOI-USGS#446's.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HBt3Ee7eneKtdKQWq2T3YK
@thodson-usgs
thodson-usgs force-pushed the fix/ogc-dates-validate-interval-strings branch from 7351697 to 12a2334 Compare October 12, 2026 20:30
@thodson-usgs
thodson-usgs merged commit b13c6b9 into DOI-USGS:main Oct 12, 2026
11 checks passed
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.

2 participants