feat: support datetime objects in literal instantiation #1542
Conversation
|
Thanks for the PR! Lets add a test for the example in the description with |
Added, let me know if we want anything more explicit there |
tests/integration/test_reads.py
Outdated
| def test_scan_with_datetime(catalog: Catalog) -> None: | ||
| table = create_table(catalog) | ||
| # test that this doesn't raise an exception... | ||
| table.scan(row_filter=GreaterThanOrEqual("datetime", datetime.now())).to_pandas() |
There was a problem hiding this comment.
nit: the table has datetime column, what if we add a row and then filter datetime before and after and assert the results.
maybe something like,
yesterday = ...
table.append({"datatime": yesterday)
assert len(table.scan(row_filter=GreaterThanOrEqual("datetime", datetime.now()))) == 0
assert len(table.scan(row_filter=LessThanOrEqual("datetime", datetime.now()))) == 1
There was a problem hiding this comment.
thanks, looking much better now, took me too long to figure out I can run make test-integration PYTEST_ARGS='-k test_scan_with_datetime' to only select that test hahaha
Fokko
left a comment
There was a problem hiding this comment.
Looks good, thanks for adding this @jayceslesar 🙌 Thanks for the review @kevinjqliu 🚀
pyiceberg/expressions/literals.py
Outdated
| elif isinstance(value, Decimal): | ||
| return DecimalLiteral(value) | ||
| elif isinstance(value, datetime): | ||
| return TimestampLiteral(datetime_to_micros(value)) |
There was a problem hiding this comment.
The tzinfo is handled here:
iceberg-python/pyiceberg/utils/datetime.py
Lines 75 to 81 in 3981e62
|
this introduces sort of a tricky typing problem... Will check it out tonight but can't just fix by adding |
|
Thanks for the contribution @jayceslesar! and thanks for the review @Fokko :) |
Support
datetimeobjects being passed into theliteralfunction.Allows situations like the following:
Which previously failed
Closes #1456