-
Notifications
You must be signed in to change notification settings - Fork 76
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
refactor: improve periods module doctests, documentation, and type hints #1043
Conversation
@@ -0,0 +1,259 @@ | |||
from __future__ import annotations | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@benjello My idea to abstract date units from periods, so to facilitate their extension (weeks and so on), based on indexed enums. What do you think?
|
||
""" | ||
|
||
return DateUnit.DAY, DateUnit.MONTH, DateUnit.YEAR |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If these were int
rather than str
, we could use bitwise operations, I guess that could help vectorize period operations?
|
||
.. versionadded:: 35.9.0 | ||
|
||
""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This has two benefits:
-
We consolidate the definition of date units in just one place, instead of using literals everywhere.
-
We can, at some point, use
DateUnit.Day.index
instead ofday
orDateUnit.value
. So to be able to pass from:
if period.unit == "eternity"
period.unit in ["eternirty", ...]
To :
allowed_periods = DateUnit.DAY | DateUnit.MONTH
definition_period = [0, 1, 0, 0]
isperiod = definition_period[allowed_periods]
6f973ca
to
bfe9e56
Compare
5c19d73
to
93bd7ef
Compare
3539db1
to
1e88559
Compare
970b036
to
e48eaaa
Compare
1e88559
to
af3636b
Compare
e48eaaa
to
0193f93
Compare
Part of #1061