-
Notifications
You must be signed in to change notification settings - Fork 3.2k
Duration must be a timedelta instead of string #19427
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
Changes from all commits
6658474
0c4525d
fc2aa93
4bdfe3f
021ae7a
0cf9ac9
e18aacb
e0ddb73
427b3b1
7da5472
d0dcbfa
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -50,17 +50,23 @@ def order_results(request_order, responses): | |
| return ordered | ||
|
|
||
| def construct_iso8601(start=None, end=None, duration=None): | ||
| if duration is not None: | ||
| duration = 'PT{}S'.format(duration.total_seconds()) | ||
| iso_str = None | ||
| if start is not None: | ||
| start = Serializer.serialize_iso(start) | ||
| if end and duration: | ||
| raise ValueError("start_time can only be provided with duration or end_time, but not both.") | ||
| if end is not None: | ||
| end = Serializer.serialize_iso(end) | ||
| iso_str = start + '/' + end | ||
| elif duration is not None: | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What's the behavior if user provides start, end and duration?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we would take the start and end time - we have documented that all three should not be provided - on retrospect, let me warn here too
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In fact my question is if three are provided, we would silently ignore duration (current behavior) or give error? :)
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we now give error :) |
||
| iso_str = start + '/' + duration | ||
| else: | ||
| raise ValueError("Start time must be provided aling with duration or end time.") | ||
| raise ValueError("Start time must be provided along with duration or end time.") | ||
| elif end is not None: | ||
| if not duration: | ||
| raise ValueError("End time must be provided along with duration or start time.") | ||
| end = Serializer.serialize_iso(end) | ||
| iso_str = duration + '/' + end | ||
| else: | ||
|
|
||
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.
Should we use isinstance?
If user uses a class that has total_seconds property, is it valid?
Uh oh!
There was an error while loading. Please reload this page.
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.
What if duration is None in line 69 & 71?
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.
changed to
if duration is not NoneThere 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.
Just to confirm, you mean
is valid?
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.
oops - line 69-71 got changed when i made the changes :)
nope =
iso_str = None + '/' + endis not a valid format - although end must be provided with a duration