redis-om-python
redis-om-python copied to clipboard
Support time points as numeric indexes
This is a rough draft
What's is implemented:
datetime.datetimeworks as numeric index.datetime.dateworks as numeric index.datetime.timeworks as numeric index. ~~(very sketchy, see below)~~- Add test for all above for:
- Saving to Redis, checking if is valid.
- Finding by less operator(any comparison should work) in sorted fashion.
- Saving no-UTC and checking that restored version(UTC) is equal to pre-saving no-UTC value.
What can be added / improved:
- ~~
datetime.timecustom parsing implementation~~ - Our type for
datetime.timebecause user will have to write parsing(validation) themselves otherwise(seepost_model_timein tests). - Support for
datetime.timedelta(I skipped it for now). - Support for
Model.find(Model.field < "Thu Sep 25 10:36:28 2003")or ISO format, or etc... - Documentation.
- More tests.
- MORE TESTS(Seriously, I don't think I understand time zones 100%).
Questions:
- How to store timezone information as numeric field? Right now I just convert everything to UTC before saving. (This needs to be checked)
- What to do if user did not provide timezone?
- Do we want to support
Model.find(Model.field < "Thu Sep 25 10:36:28 2003")or"2003-09-25T10:49:41.5-03:00"or any other string in expressions. (Becausepydanticsupports only ISO format(I think)). We could usedateutil. Or make it optional dependency. - Precision. To what precision should we save time-points. I implemented milliseconds because:
- It should be enough
pydanticparsing support parsingdatetimefrom integers ms out of the box.- Later I checked
redis-om-node, and there they support precision to seconds(I'm not sure): https://github.com/redis/redis-om-node#searching-on-dates
- Cross type expression(
Model.datetime < date/time). Should we support this? Or catch this and raise exception? - Will Redis at some point add "Timepoint" index field type(like NUMERIC)? (with timezone support). Because that would simplify things.
- I probably forgot thing or 10, will add later
Problems:
- datetime.time: pydantic support parsing only as:
so no int parsing ~~, very custom logic(probably wrong)~~time, existing time object str, following formats work: HH:MM[:SS[.ffffff]][Z or [±]HH[:]MM]]]
Sketchy things:
- ~~datetime.time custom parsing implementation~~
- I added this to
resolve_value. Maybe there is better approach.
https://github.com/redis/redis-om-python/pull/240/files#diff-ed42e1a9e452289cee8007ec1dc71beb5e883a54f7f58db63320901b9e4baaf3R546elif field_type is RediSearchFieldTypes.NUMERIC: + value = jsonable_encoder(value)
Fixes #199
Codecov Report
Merging #240 (97c617c) into main (c2339b3) will increase coverage by
0.36%. The diff coverage is100.00%.
@@ Coverage Diff @@
## main #240 +/- ##
==========================================
+ Coverage 77.25% 77.61% +0.36%
==========================================
Files 12 12
Lines 1165 1184 +19
==========================================
+ Hits 900 919 +19
Misses 265 265
| Flag | Coverage Δ | |
|---|---|---|
| unit | 77.61% <100.00%> (+0.36%) |
:arrow_up: |
Flags with carried forward coverage won't be shown. Click here to find out more.
| Impacted Files | Coverage Δ | |
|---|---|---|
| aredis_om/model/encoders.py | 59.77% <100.00%> (+8.38%) |
:arrow_up: |
| aredis_om/model/model.py | 86.33% <100.00%> (+0.06%) |
:arrow_up: |
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact),ø = not affected,? = missing dataPowered by Codecov. Last update c2339b3...97c617c. Read the comment docs.
Any input will be appreciated