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
Differentiate between datetimeformat and dateformat #869
Changes from 1 commit
8efb79a
0a75653
00ea44e
baa1658
b2ad73d
dcf7967
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 |
---|---|---|
|
@@ -915,14 +915,14 @@ class DateTime(Field): | |
|
||
""" | ||
|
||
DATEFORMAT_SERIALIZATION_FUNCS = { | ||
SERIALIZATION_FUNCS = { | ||
'iso': utils.isoformat, | ||
'iso8601': utils.isoformat, | ||
'rfc': utils.rfcformat, | ||
'rfc822': utils.rfcformat, | ||
} | ||
|
||
DATEFORMAT_DESERIALIZATION_FUNCS = { | ||
DESERIALIZATION_FUNCS = { | ||
'iso': utils.from_iso_datetime, | ||
'iso8601': utils.from_iso_datetime, | ||
'rfc': utils.from_rfc, | ||
|
@@ -931,62 +931,74 @@ class DateTime(Field): | |
|
||
DEFAULT_FORMAT = 'iso' | ||
|
||
OBJ_TYPE = 'datetime' | ||
|
||
SCHEMA_OPTS_VAR_NAME = 'datetimeformat' | ||
|
||
localtime = False | ||
default_error_messages = { | ||
'invalid': 'Not a valid datetime.', | ||
'format': '"{input}" cannot be formatted as a datetime.', | ||
'invalid': 'Not a valid {obj_type}.', | ||
'format': '"{input}" cannot be formatted as a {obj_type}.', | ||
} | ||
|
||
def __init__(self, format=None, **kwargs): | ||
super(DateTime, self).__init__(**kwargs) | ||
# Allow this to be None. It may be set later in the ``_serialize`` | ||
# or ``_desrialize`` methods This allows a Schema to dynamically set the | ||
# dateformat, e.g. from a Meta option | ||
self.dateformat = format | ||
# format, e.g. from a Meta option | ||
self.format = format | ||
|
||
def _add_to_schema(self, field_name, schema): | ||
super(DateTime, self)._add_to_schema(field_name, schema) | ||
self.dateformat = self.dateformat or schema.opts.dateformat | ||
self.format = self.format or \ | ||
getattr(schema.opts, self.SCHEMA_OPTS_VAR_NAME) or \ | ||
self.DEFAULT_FORMAT | ||
|
||
def _serialize(self, value, attr, obj): | ||
self.format = self.format or self.DEFAULT_FORMAT | ||
if value is None: | ||
return None | ||
dateformat = self.dateformat or self.DEFAULT_FORMAT | ||
format_func = self.DATEFORMAT_SERIALIZATION_FUNCS.get(dateformat, None) | ||
format_func = self.SERIALIZATION_FUNCS.get(self.format, None) | ||
if format_func: | ||
try: | ||
return format_func(value, localtime=self.localtime) | ||
except (AttributeError, ValueError): | ||
self.fail('format', input=value) | ||
except (TypeError, AttributeError, ValueError): | ||
self.fail('format', input=value, obj_type=self.OBJ_TYPE) | ||
else: | ||
return value.strftime(dateformat) | ||
return value.strftime(self.format) | ||
|
||
def _deserialize(self, value, attr, data): | ||
self.format = self.format or self.DEFAULT_FORMAT | ||
if not value: # Falsy values, e.g. '', None, [] are not valid | ||
raise self.fail('invalid') | ||
dateformat = self.dateformat or self.DEFAULT_FORMAT | ||
func = self.DATEFORMAT_DESERIALIZATION_FUNCS.get(dateformat) | ||
raise self.fail('invalid', obj_type=self.OBJ_TYPE) | ||
func = self.DESERIALIZATION_FUNCS.get( | ||
self.format, | ||
) | ||
if func: | ||
try: | ||
return func(value) | ||
except (TypeError, AttributeError, ValueError): | ||
raise self.fail('invalid') | ||
elif dateformat: | ||
raise self.fail('invalid', obj_type=self.OBJ_TYPE) | ||
elif self.format: | ||
try: | ||
return dt.datetime.strptime(value, dateformat) | ||
return dt.datetime.strptime(value, self.format) | ||
except (TypeError, AttributeError, ValueError): | ||
raise self.fail('invalid') | ||
raise self.fail('invalid', obj_type=self.OBJ_TYPE) | ||
elif utils.dateutil_available: | ||
try: | ||
return utils.from_datestring(value) | ||
except TypeError: | ||
raise self.fail('invalid') | ||
parsed = utils.from_datestring(value) | ||
return dt.datetime( | ||
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 don't understand this change. I don't think this would return a Isn't this code unreachable anyway ? (#758) (I don't mean to throw #758 in your way, this is another story. If the code is indeed unreachable, which I think it is, don't worry too much about it. I just don't understand the change.) 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. The object creation is an oversight. I'll fix that. I think #758 is beyond the scope of this PR. I can look at it after this change, though, if you need someone to work on it. |
||
parsed.year, parsed.month, parsed.day, parsed.hour, | ||
parsed.minute, parsed.second, | ||
) | ||
except (TypeError, ValueError): | ||
raise self.fail('invalid', obj_type=self.OBJ_TYPE) | ||
else: | ||
warnings.warn( | ||
'It is recommended that you install python-dateutil ' | ||
'for improved datetime deserialization.', | ||
) | ||
raise self.fail('invalid') | ||
raise self.fail('invalid', obj_type=self.OBJ_TYPE) | ||
|
||
|
||
class LocalDateTime(DateTime): | ||
|
@@ -1030,35 +1042,33 @@ def _deserialize(self, value, attr, data): | |
self.fail('invalid') | ||
|
||
|
||
class Date(Field): | ||
class Date(DateTime): | ||
"""ISO8601-formatted date string. | ||
|
||
:param format: Either ``"iso"`` (for ISO8601) or a date format string. | ||
If `None`, defaults to "iso". | ||
:param kwargs: The same keyword arguments that :class:`Field` receives. | ||
""" | ||
default_error_messages = { | ||
'invalid': 'Not a valid date.', | ||
'format': '"{input}" cannot be formatted as a date.', | ||
} | ||
|
||
def _serialize(self, value, attr, obj): | ||
if value is None: | ||
return None | ||
try: | ||
return value.isoformat() | ||
except AttributeError: | ||
self.fail('format', input=value) | ||
return value | ||
SERIALIZATION_FUNCS = { | ||
'iso': utils.to_iso_date, | ||
'iso8601': utils.to_iso_date, | ||
} | ||
|
||
def _deserialize(self, value, attr, data): | ||
"""Deserialize an ISO8601-formatted date string to a | ||
:class:`datetime.date` object. | ||
""" | ||
if not value: # falsy values are invalid | ||
self.fail('invalid') | ||
try: | ||
return utils.from_iso_date(value) | ||
except (AttributeError, TypeError, ValueError): | ||
self.fail('invalid') | ||
DESERIALIZATION_FUNCS = { | ||
'iso': utils.from_iso_date, | ||
'iso8601': utils.from_iso_date, | ||
} | ||
|
||
DEFAULT_FORMAT = 'iso' | ||
|
||
OBJ_TYPE = "date" | ||
|
||
SCHEMA_OPTS_VAR_NAME = 'dateformat' | ||
|
||
|
||
class TimeDelta(Field): | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1468,19 +1468,32 @@ class BadExclude(Schema): | |
class Meta: | ||
exclude = 'name' | ||
|
||
def test_dateformat_option(user): | ||
def test_datetimeformat_option(user): | ||
fmt = '%Y-%m' | ||
|
||
class DateFormatSchema(Schema): | ||
class DateTimeFormatSchema(Schema): | ||
updated = fields.DateTime('%m-%d') | ||
|
||
class Meta: | ||
fields = ('created', 'updated') | ||
dateformat = fmt | ||
serialized = DateFormatSchema().dump(user) | ||
datetimeformat = fmt | ||
serialized = DateTimeFormatSchema().dump(user) | ||
assert serialized['created'] == user.created.strftime(fmt) | ||
assert serialized['updated'] == user.updated.strftime('%m-%d') | ||
|
||
def test_dateformat_option(user): | ||
fmt = '%Y-%m' | ||
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. Very minor: it is strange to define this format here and no the other one. 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 was the pattern saw I above at line 1471. I'll go ahead name them in both tests. 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. Oh, right, of course. Thanks. No pb otherwise. |
||
|
||
class DateFormatSchema(Schema): | ||
birthdate = fields.Date('%m-%d') | ||
|
||
class Meta: | ||
fields = ('birthdate', 'activation_date') | ||
dateformat = fmt | ||
serialized = DateFormatSchema().dump(user) | ||
assert serialized['birthdate'] == user.birthdate.strftime('%m-%d') | ||
assert serialized['activation_date'] == user.activation_date.strftime(fmt) | ||
|
||
def test_default_dateformat(user): | ||
class DateFormatSchema(Schema): | ||
updated = fields.DateTime(format='%m-%d') | ||
|
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 will conflict with #934.
Please don't mutate
self.format
and use a localformat
variable.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.
Okay. It seems that change was merged just a few days ago. I'll go ahead and change
self.format
to justformat
in this case. Do you want me to change the name of theself.format
variable back toself.dateformat
as well? I changed it sincedateformat
in theDateTime
field doesn't make sense.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.
Yes, exactly. I though I'd mention that to help you with the rebase.
No, not at all.
format
is better now.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.
Sorry, I just realized this PR is rebased already. Then please change
self.format
toformat
as in #934.