Skip to content

Fix end-only measurement date filters - #126

Open
SuhrudhC wants to merge 1 commit into
openaq:mainfrom
SuhrudhC:fix/end-only-measurement-filters
Open

SuhrudhC wants to merge 1 commit into
openaq:mainfrom
SuhrudhC:fix/end-only-measurement-filters

Conversation

@SuhrudhC

@SuhrudhC SuhrudhC commented Oct 7, 2026

Copy link
Copy Markdown

Problem and fix

Fixes #125.

An ordinary client.measurements.list(sensors_id=1, data="days", date_to="2024-01-01") raises InvalidParameterError because the validator also requires date_from. End-only datetime filters fail similarly. Both bounds are optional in the documented public method and supported independently by the API.

Validate supplied end bounds and return them when the start is absent. Retain two-bound ordering checks and validate falsey date ends instead of silently dropping them. The change is confined to date-filter validation and small public-client regressions; it introduces no constructor options or transport changes.

Validation

  • Eight end-only client cases fail on unmodified main and the pending v1.2.0 branch, and pass with the patch: all four data modes, strings and date/datetime objects.
  • Four invalid-end cases verify rejection before dispatch.
  • Full main unit suite on Python 3.12: 1,085 passed.
  • Repository Ruff lint/format and mypy checks: pass.
  • Pending v1.2.0 client/validator modules with the source patch: 310 passed, including timezone regressions. Porting the test file needed its existing _has_toml import removed to match that branch; the source patch applied cleanly.

Targets main per CONTRIBUTING.md. The issue is also present on v1.2.0; I can retarget if preferred. Generated and validated with Codex. Tests use the standard public client and stub outgoing HTTP responses; no _transport injection, real credentials, or live API requests are required.

@codecov-commenter

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.31%. Comparing base (fd82385) to head (41d781d).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #126      +/-   ##
==========================================
+ Coverage   95.01%   95.31%   +0.30%     
==========================================
  Files          19       19              
  Lines        1363     1367       +4     
  Branches      201      203       +2     
==========================================
+ Hits         1295     1303       +8     
+ Misses         26       23       -3     
+ Partials       42       41       -1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@russbiggs

Copy link
Copy Markdown
Member

Thanks for reporting this and submitting the PR. The fix itself looks right and date_to alone filters should be valid for this particular endpoint combination per the API spec.

Before reviewing further could you tell me a bit about how this PR came together? The description mentions "Generated by Codex", how much of the change and tests you wrote or reviewed yourself versus how much was generated?

@SuhrudhC please reply in your own words (not through Codex) with how you came across this issue and whether you've run into it in your own use of the client? That context helps us prioritize, and it helps us know who we're working with on the review.

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.

Measurements.list rejects end-only date and datetime filters

3 participants