feat: support native python types for operators#3
feat: support native python types for operators#3weiliddat merged 1 commit intoweiliddat:feat/coerce-python-typesfrom
Conversation
|
@gersmann oh cool, on first glance it makes a lot of sense, essentially providing |
Nope, not urgent, thank you. |
weiliddat
left a comment
weiliddat
left a comment
There was a problem hiding this comment.
Overall idea makes sense, so I'm gonna merge it to a branch; I have a couple of style/preference things that I'll fix and you can also do a final check that it's still working as expected
| return operator.le(doc, ov) | ||
|
|
||
| try: | ||
| return doc <= ov |
| # Regex special-case takes precedence | ||
| if isinstance(ov, re.Pattern) and isinstance(doc, str): | ||
| if ov.search(doc): | ||
| return True | ||
| return bool(ov.search(doc)) |
There was a problem hiding this comment.
just thinking, should regexes against dates / uuids (namespaced or timestamped uuids, a few coming in python 3.14) be considered?
we could coerce before the regex check and if the ov is an instance of regex, we coerce doc to string?
| # Datetime ↔︎ ISO 8601 string | ||
| def _parse_date(s: str, target): | ||
| try: | ||
| if target is datetime.date: | ||
| return datetime.date.fromisoformat(s) | ||
| return datetime.datetime.fromisoformat(s) | ||
| except ValueError: | ||
| # Not a valid ISO format – leave as string so normal | ||
| # comparison semantics apply (and tests expecting a failure | ||
| # still pass). | ||
| return s |
There was a problem hiding this comment.
style: move _parse_date out to module scope
| # UUID ↔︎ string | ||
| def _to_uuid(val: object): | ||
| if isinstance(val, uuid.UUID): | ||
| return val | ||
| if isinstance(val, str): | ||
| try: | ||
| return uuid.UUID(val) | ||
| except ValueError: | ||
| return val | ||
| return val |
There was a problem hiding this comment.
style: move _to_uuid out to module scope
|
|
||
| # Only attempt Decimal coercion when at least *one* operand is already a | ||
| # Decimal instance. | ||
| if isinstance(a, decimal.Decimal) or isinstance(b, decimal.Decimal): |
There was a problem hiding this comment.
style: same XOR comparison like uuid? _try_decimal also checks/returns the value without conversion if the value is a decimal
| if isinstance(doc, list) and any([_match_gt(d, path, ov) for d in doc]): | ||
| return True | ||
|
|
||
| doc, ov = coerce(doc, ov) |
There was a problem hiding this comment.
I'd move it to after the list/dict checks since those are pretty specific checks that are pretty unlikely to interact with coerced types
Thanks CW, will be pointing our branch to your latest. |
2 participants
Hi @weiliddat, hope you're doing well.
Question - would you be open to this extension?
This makes it so that when there are native objects in the doc, that have a different JSON representation (datetime/date/decimal/uuid in this PR), and the filter value is parsable as native object, the filter value gets coerced before comparing.
This allows serializing the filters as JSON (for persistence), and still having 'valid' results in query evaluation. Some work-arounds are possible, e.g. for
datetime, the doc could be roundtripped to-from JSON, but for decimals for instance, this work-around doesn't work (because "9" > "10".