piccolo
piccolo copied to clipboard
`Timestamptz` autoconversion
Based off https://github.com/piccolo-orm/piccolo/pull/959
Created this branch for testing. Experimenting with the AT TIME ZONE clause.
Remaining tasks:
- [x] Get
selectandobjectsqueries working - [x] Get SQLite working
- [x] Fix existing tests
- [ ] Add docs for
at_time_zonemethod - [ ] Add tests for
selectqueries - [ ] Add tests for the
at_time_zonemethod - [x] Optimise
Objects._get_select_querya bit - [ ] Add an
at_time_zonemethod toobjects
Codecov Report
Attention: Patch coverage is 94.89796% with 5 lines in your changes are missing coverage. Please review.
Project coverage is 92.77%. Comparing base (
363d683) to head (2c75e70). Report is 1 commits behind head on master.
| Files | Patch % | Lines |
|---|---|---|
| piccolo/columns/column_types.py | 81.81% | 4 Missing :warning: |
| piccolo/query/methods/objects.py | 96.15% | 1 Missing :warning: |
:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.
Additional details and impacted files
@@ Coverage Diff @@
## master #978 +/- ##
==========================================
- Coverage 92.78% 92.77% -0.02%
==========================================
Files 108 109 +1
Lines 8182 8216 +34
==========================================
+ Hits 7592 7622 +30
- Misses 590 594 +4
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
I think this is almost done now. select queries work fine. But surprisingly object queries override response_handler from select, so aren't currently working. It's just a matter of shuffling some logic around.
Update: objects queries now work too.
Now we need to work out what to do with SQLite - I might just raise a ValueError for now.
Update: I was able to get SQLite working.
@dantownsend I tried your solution and everything worked great. Here is a Piccolo shell results
# if we declare a time zone in the table definition
# class TallinnConcerts(Table):
# event_start = Timestamptz(at_time_zone=ZoneInfo("Europe/Tallinn"))
In [1]: await TallinnConcerts.select()
Out[1]:
[{'id': 1,
'event_start': datetime.datetime(2024, 4, 5, 10, 44, 46, tzinfo=zoneinfo.ZoneInfo(key='Europe/Tallinn'))},
{'id': 2,
'event_start': datetime.datetime(2024, 4, 5, 10, 44, 46, tzinfo=zoneinfo.ZoneInfo(key='Europe/Tallinn'))}]
In [2]: [i.to_dict() for i in await TallinnConcerts.objects()]
Out[2]:
[{'id': 1,
'event_start': datetime.datetime(2024, 4, 5, 10, 44, 46, tzinfo=zoneinfo.ZoneInfo(key='Europe/Tallinn'))},
{'id': 2,
'event_start': datetime.datetime(2024, 4, 5, 10, 44, 46, tzinfo=zoneinfo.ZoneInfo(key='Europe/Tallinn'))}]
# if we do not declare a timezone in the table definition, we can define `at_time_zone` in the select query
# class TallinnConcerts(Table):
# event_start = Timestamptz()
In [3]: await TallinnConcerts.select(TallinnConcerts.event_start.at_time_zone("Europe/Tallinn"))
Out[3]:
[{'event_start': datetime.datetime(2024, 4, 6, 16, 37, 33, tzinfo=zoneinfo.ZoneInfo(key='Europe/Tallinn'))},
{'event_start': datetime.datetime(2024, 4, 6, 16, 37, 33, tzinfo=zoneinfo.ZoneInfo(key='Europe/Tallinn'))}]
Piccolo Admin also work.
@sinisaos Thanks a lot for testing it.
What do you think about the design of it?
I changed it to be:
Timestamptz(at_time_zone=ZoneInfo("America/New_York"))
So the argument is at_time_zone, instead of tz to be consistent with the at_time_zone method name, and to make it clearer what it does.
I was thinking of adding a method to the objects query too. For select we can do this:
await Concert.select(Concert.starts_at.at_time_zone('America/Los_Angeles'))
But there's currently no way of overriding it for objects queries. Maybe something like:
await Concert.objects().at_time_zone({Concert.starts_at: 'America/Los_Angeles'})
@dantownsend Everything looks and works great to me, as I wrote in the previous comment.
But there's currently no way of overriding it for
objectsqueries. Maybe something like:await Concert.objects().at_time_zone({Concert.starts_at: 'America/Los_Angeles'})
It would be great to have that objects method to manipulate the results.
@sinisaos Thanks a lot for testing it.
What do you think about the design of it?
I changed it to be:
Timestamptz(at_time_zone=ZoneInfo("America/New_York"))So the argument is
at_time_zone, instead oftzto be consistent with theat_time_zonemethod name, and to make it clearer what it does.I was thinking of adding a method to the
objectsquery too. For select we can do this:await Concert.select(Concert.starts_at.at_time_zone('America/Los_Angeles'))But there's currently no way of overriding it for
objectsqueries. Maybe something like:await Concert.objects().at_time_zone({Concert.starts_at: 'America/Los_Angeles'})
I quite like this polars-like select expression (or should I say polars is piccolo-like 😊?), which feels natural to use. Additionally, I agree with @sinisaos that the syntax of the objects method looks good to me.