feat(python)!: one filter shape for every service - #104
Merged
Conversation
`filter()` took three different shapes depending on the entity. Events and datasets wanted an
envelope — `EventFilterForm(filter=EventFilter(...), limit=100)`. Timeseries wanted
`TimeSeriesFilterForm`, which despite the name *was* the criteria, with paging flattened in beside
it and no `TimeSeriesFilter` to nest. Resources took bare keywords and no filter object at all,
even though `resources.search` accepted one. Someone who learned the events API and wrote the
obvious analogue for timeseries got a NameError for the class that does not exist and a TypeError
for the `filter=` keyword.
All four now take either the criteria as keywords or a prepared `filter=` object, with paging
always on the call:
client.timeseries.filter(name=["Pump*"], limit=100)
PUMPS = TimeSeriesFilter(name=["Pump*"])
client.timeseries.filter(filter=PUMPS, limit=100, sort_by="name")
client.timeseries.search("vibration", filter=PUMPS)
Passing both a filter and criteria keywords is a TypeError rather than a merge: that call has two
intents in it, and either resolution — keyword wins, or union — discards one of them silently.
Keeping paging out of the filter fixes a real defect rather than only tidying names. Because
timeseries had no criteria class, `timeseries.search` accepted the *form* and reached inside it for
the criteria, so every paging field on a form handed to `search` was discarded without a word —
`search(q, filter=TimeSeriesFilterForm(name=[...], limit=5), limit=100)` silently ignored the 5.
A filter that cannot hold a limit cannot lose one, and the same object is now safe to reuse across
`filter()` and `search()` without carrying a stale cursor into its next use.
`TimeSeriesFilterForm`, `EventFilterForm` and `DatasetFilterForm` are gone from the bindings rather
than kept as deprecated aliases. 0.2.0 is a day old with no adoption to protect, and an alias would
preserve exactly the ambiguity that caused the bug. Rust is untouched: `XFilterForm` is the wire
body there and stays, which AGENTS.md now states explicitly instead of leaving the Python surface
to be inferred from the Rust convention.
Subscriptions are deliberately out of scope: `subscriptions.list` is a different call with its own
contract, and half-converting it would leave a fourth shape rather than removing one.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
filter()took three different shapes depending on the entity. Events and datasets wanted an envelope —EventFilterForm(filter=EventFilter(...), limit=100). Timeseries wantedTimeSeriesFilterForm, which despite the name was the criteria, with paging flattened in beside it and noTimeSeriesFilterto nest. Resources took bare keywords and no filter object at all, even thoughresources.searchaccepted one. Someone who learned the events API and wrote the obvious analogue for timeseries got a NameError for the class that does not exist and a TypeError for thefilter=keyword.All four now take either the criteria as keywords or a prepared
filter=object, with paging always on the call:Passing both a filter and criteria keywords is a TypeError rather than a merge: that call has two intents in it, and either resolution — keyword wins, or union — discards one of them silently.
Keeping paging out of the filter fixes a real defect rather than only tidying names. Because timeseries had no criteria class,
timeseries.searchaccepted the form and reached inside it for the criteria, so every paging field on a form handed tosearchwas discarded without a word —search(q, filter=TimeSeriesFilterForm(name=[...], limit=5), limit=100)silently ignored the 5. A filter that cannot hold a limit cannot lose one, and the same object is now safe to reuse acrossfilter()andsearch()without carrying a stale cursor into its next use.TimeSeriesFilterForm,EventFilterFormandDatasetFilterFormare gone from the bindings rather than kept as deprecated aliases. 0.2.0 is a day old with no adoption to protect, and an alias would preserve exactly the ambiguity that caused the bug. Rust is untouched:XFilterFormis the wire body there and stays, which AGENTS.md now states explicitly instead of leaving the Python surface to be inferred from the Rust convention.Subscriptions are deliberately out of scope:
subscriptions.listis a different call with its own contract, and half-converting it would leave a fourth shape rather than removing one.