-
-
Notifications
You must be signed in to change notification settings - Fork 152
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
Added get_relative_delta
and get_tzinfo
in datetime API
#2310
Conversation
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.
Cool, the dateutil
seems to provide some mighty functionalities.
Is it a dependency of Plone or who actually requires it?
Only some minor changes required before we can merge it.
src/senaite/core/api/dtime.py
Outdated
def get_relative_delta(from_dtime, to_dtime=None): | ||
"""Returns the relative delta between two datetimes. If to_dtime is None, | ||
compares current datetime with from_dtime | ||
""" |
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.
Would you mind to provide the docstrings in API functions in the same way we did for the to_localized_time
, e.g.:
def get_relative_delta(from_dtime, to_dtime=None):
"""Calculate the relative delta between two dates
If `to_dtime` is None, the current datetime is used.
:param from_dtime:
:type from_dtime: str/datetime/DateTime
:param to_dtime:
:type to_dtime: str/datetime/DateTime
:returns: relativedelta object, e.g. `relativedelta(hours=+3)`
:rtype: object
"""
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.
Fixed with 0aed612
src/senaite/core/api/dtime.py
Outdated
from_dtime = to_dt(from_dtime) | ||
to_dtime = to_dt(to_dtime) | ||
if not all([from_dtime, to_dtime]): | ||
return None |
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.
Better raise an Exception if the parameters can not be converted to valid dates.
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.
Done with ec73794
src/senaite/core/api/dtime.py
Outdated
try: | ||
return pytz.timezone(get_timezone(dt)) | ||
except pytz.UnknownTimeZoneError: | ||
return default |
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.
Please make this an own API function (outside of this function)
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.
Done with 5e4f661
get_relative_delta
and get_tzinfo
in datetime API
Is used by some plone.* packages, like |
Description of the issue/feature this PR addresses
This Pull Request adds the function
get_relative_delta
in dtime apiCurrent behavior before PR
There is no handy function to get the relative delta between two dates
Desired behavior after PR is merged
There is a handy function to get the relative delta between two dates
--
I confirm I have tested this PR thoroughly and coded it according to PEP8
and Plone's Python styleguide standards.