-
-
Notifications
You must be signed in to change notification settings - Fork 7k
DurationField output format #8532
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
base: master
Are you sure you want to change the base?
Changes from 1 commit
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 |
---|---|---|
|
@@ -377,13 +377,16 @@ A Duration representation. | |
Corresponds to `django.db.models.fields.DurationField` | ||
|
||
The `validated_data` for these fields will contain a `datetime.timedelta` instance. | ||
The representation is a string following this format `'[DD] [HH:[MM:]]ss[.uuuuuu]'`. | ||
|
||
**Signature:** `DurationField(max_value=None, min_value=None)` | ||
**Signature:** `DurationField(format=api_settings.DURATION_FORMAT, max_value=None, min_value=None)` | ||
|
||
* `format` - A string representing the output format. If not specified, this defaults to the same value as the `DURATION_FORMAT` settings key, which will be `'standard'` unless set. Setting to a format string indicates that `to_representation` return values should be coerced to string output. Format strings are described below. Setting this value to `None` indicates that Python `timedelta` objects should be returned by `to_representation`. In this case the date encoding will be determined by the renderer. | ||
* `max_value` Validate that the duration provided is no greater than this value. | ||
* `min_value` Validate that the duration provided is no less than this value. | ||
|
||
#### `DurationField` format strings | ||
Format strings may either be the special string `'iso-8601'`, which indicates that [ISO 8601][iso8601] style intervals should be used (eg `'P4DT1H15M20S'`), or the special string `'standard'`, which indicates that Django interval format `'[DD] [HH:[MM:]]ss[.uuuuuu]'` should be used (eg: `'4 1:15:20'`). | ||
Comment on lines
+383
to
+388
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. I find the use of the term "format string" very confusing here. The term means something else in Python, referring to string with placeholder variables that get filled in. That sounds like one could set 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. Good point, how about 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. sounds good! 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. I used the "format" name just to keep uniformity with date/time fields, because to me it looked more elegant. Since there is nothing like strftime/strptime for |
||
|
||
--- | ||
|
||
# Choice selection fields | ||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -314,6 +314,15 @@ May be a list including the string `'iso-8601'` or Python [strftime format][strf | |||||
|
||||||
Default: `['iso-8601']` | ||||||
|
||||||
|
||||||
#### DURATION_FORMAT | ||||||
|
||||||
A format string that should be used by default for rendering the output of `DurationField` serializer fields. If `None`, then `DurationField` serializer fields will return Python `timedelta` objects, and the duration encoding will be determined by the renderer. | ||||||
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 output style does this produce with the current JSONRenderer? 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. Using the current django-rest-framework/rest_framework/utils/encoders.py Lines 39 to 40 in 129890a
Using the same example value will be: from datetime import timedelta
d = timedelta(days=4, hours=1, minutes=15, seconds=20)
print(str(d.total_seconds())
# 350120.0 |
||||||
|
||||||
May be any of `None`, `'iso-8601'` or `'standard'` (the format accepted by `django.utils.dateparse.parse_duration`). | ||||||
|
||||||
Default: `'standard'` | ||||||
|
||||||
--- | ||||||
|
||||||
## Encodings | ||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -21,6 +21,7 @@ | |
|
||
# Default datetime input and output formats | ||
ISO_8601 = 'iso-8601' | ||
STD_DURATION = 'standard' | ||
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. It turns out that this value isn't used anywhere; as long as That's probably not intended. Can this be correct? Let's also add a test case for the behavior with an unknown 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. That value was placed just to be a placeholder for the default format. It may just be set to It is true that django itself does not provide much flexibility with duration representation, which also comes from stdlib Which way do you prefer for completing this? |
||
|
||
|
||
class RemovedInDRF317Warning(PendingDeprecationWarning): | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -24,7 +24,7 @@ | |
from django.utils.dateparse import ( | ||
parse_date, parse_datetime, parse_duration, parse_time | ||
) | ||
from django.utils.duration import duration_string | ||
from django.utils.duration import duration_iso_string, duration_string | ||
from django.utils.encoding import is_protected_type, smart_str | ||
from django.utils.formats import localize_input, sanitize_separators | ||
from django.utils.ipv6 import clean_ipv6_address | ||
|
@@ -1351,9 +1351,11 @@ class DurationField(Field): | |
'overflow': _('The number of days must be between {min_days} and {max_days}.'), | ||
} | ||
|
||
def __init__(self, **kwargs): | ||
def __init__(self, format=empty, **kwargs): | ||
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 should disallow specifying this as positional argument: All other options come from the |
||
self.max_value = kwargs.pop('max_value', None) | ||
self.min_value = kwargs.pop('min_value', None) | ||
if format is not empty: | ||
self.format = format | ||
super().__init__(**kwargs) | ||
if self.max_value is not None: | ||
message = lazy_format(self.error_messages['max_value'], max_value=self.max_value) | ||
|
@@ -1376,6 +1378,13 @@ def to_internal_value(self, value): | |
self.fail('invalid', format='[DD] [HH:[MM:]]ss[.uuuuuu]') | ||
|
||
def to_representation(self, value): | ||
output_format = getattr(self, 'format', api_settings.DURATION_FORMAT) | ||
|
||
if output_format is None or isinstance(value, str): | ||
return value | ||
|
||
if output_format.lower() == ISO_8601: | ||
return duration_iso_string(value) | ||
return duration_string(value) | ||
|
||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1773,8 +1773,31 @@ class TestDurationField(FieldValues): | |
field = serializers.DurationField() | ||
|
||
|
||
# Choice types... | ||
class TestNoOutputFormatDurationField(FieldValues): | ||
""" | ||
Values for `TimeField` with a no output format. | ||
auvipy marked this conversation as resolved.
Show resolved
Hide resolved
|
||
""" | ||
valid_inputs = {} | ||
invalid_inputs = {} | ||
outputs = { | ||
datetime.timedelta(1): datetime.timedelta(1) | ||
} | ||
field = serializers.DurationField(format=None) | ||
|
||
|
||
class TestISOOutputFormatDurationField(FieldValues): | ||
""" | ||
Values for `TimeField` with a custom output format. | ||
auvipy marked this conversation as resolved.
Show resolved
Hide resolved
|
||
""" | ||
valid_inputs = {} | ||
invalid_inputs = {} | ||
Comment on lines
+1792
to
+1793
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. Would be nice to add test coverage for parsing duration in ISO format, as I would expect it to work both ways 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. Currently the This could be added to the Where do you prefer these tests to be added? PS: 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. I'm not sure where that an extension of this error message should be considered a breaking change; the wording "one of" already hints that the application should not rely the one specific format being given in the message. |
||
outputs = { | ||
datetime.timedelta(days=3, hours=8, minutes=32, seconds=1, microseconds=123): 'P3DT08H32M01.000123S' | ||
} | ||
field = serializers.DurationField(format='iso-8601') | ||
|
||
|
||
# Choice types... | ||
class TestChoiceField(FieldValues): | ||
""" | ||
Valid and invalid values for `ChoiceField`. | ||
|
Uh oh!
There was an error while loading. Please reload this page.