refactor(postgrest): replace Any with explicit types in array filters#1534
Open
gottostartsomewhere wants to merge 1 commit into
Open
refactor(postgrest): replace Any with explicit types in array filters#1534gottostartsomewhere wants to merge 1 commit into
gottostartsomewhere wants to merge 1 commit into
Conversation
Address review on supabase#1530: instead of only coercing values at runtime, remove Any from the filter argument annotations. Add a FilterValue alias (str | int | float | bool) and type the array/collection filters (in_, cs, cd, contains, contained_by, ov, overlaps) with it, and type the JSON-object branch as Dict[str, JSON]. Runtime str-coercion is kept so integer arrays still work. mypy clean; existing tests pass.
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
1 participant
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.
Follow-up to #1530.
In the review there, @o-santi noted the proper fix is to remove
Anyfrom the filter arguments and be stricter about the types users provide. This does that for the array/collection filters.Change
FilterValue = Union[str, int, float, bool]alias intypes.py.in_,cs,cd,contains,contained_by,ov, andoverlapswithIterable[FilterValue]instead ofIterable[Any], and type the JSON-object branch ofcontains/contained_by/ovasDict[str, JSON].strcoercion (from fix(postgrest): coerce non-string values in array filters #1530) is kept, so integer arrays still work — this just makes the accepted types explicit at the type level.Checks
mypyclean on the changed files.Scope
I kept this to the array/collection filters that #1530 touched. Happy to extend the same treatment to the scalar operators (
eq,neq,gt, …, which still takevalue: Any) in this PR or a separate one — whichever you prefer. And of course feel free to adjust the target types or close this if you'd rather own the wider cleanup.