Code review comment for lp:~leonardr/launchpadlib/convert-datetime-309950

Revision history for this message
Gary Poster (gary) wrote :

*merge-conditional (Edwin)

This is a small change, but really is nice. Just about everything looks good
to me. I only have one small style niggle, below.

> === modified file 'launchpadlib/_browser.py'
> --- launchpadlib/_browser.py 2008-12-05 21:38:50 +0000
> +++ launchpadlib/_browser.py 2009-01-21 21:38:08 +0000
> @@ -43,7 +43,8 @@
> from launchpadlib.errors import HTTPError
> from launchpadlib._oauth.oauth import (
> OAuthRequest, OAuthSignatureMethod_PLAINTEXT)
> -from launchpadlib._utils import uri
> +from launchpadlib._utils import uri, json
> +from launchpadlib._utils.json import DatetimeJSONEncoder
>
>
> OAUTH_REALM = 'https://api.launchpad.net'
> @@ -261,5 +262,5 @@
> headers['If-Match'] = cached_etag
>
> self._request(
> - url, simplejson.dumps(representation), 'PATCH',
> - extra_headers=extra_headers)
> + url, simplejson.dumps(representation, cls=DatetimeJSONEncoder),
> + 'PATCH', extra_headers=extra_headers)
>
> === added file 'launchpadlib/_utils/json.py'
> --- launchpadlib/_utils/json.py 1970-01-01 00:00:00 +0000
> +++ launchpadlib/_utils/json.py 2009-01-21 21:38:08 +0000
> @@ -0,0 +1,35 @@
> +# Copyright 2009 Canonical Ltd.
> +
> +# This file is part of launchpadlib.
> +#
> +# launchpadlib is free software: you can redistribute it and/or modify
> +# it under the terms of the GNU Lesser General Public License as
> +# published by the Free Software Foundation, either version 3 of the
> +# License, or (at your option) any later version.
> +#
> +# launchpadlib is distributed in the hope that it will be useful, but
> +# WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> +# Lesser General Public License for more details.
> +#
> +# You should have received a copy of the GNU Lesser General Public
> +# License along with launchpadlib. If not, see
> +# <http://www.gnu.org/licenses/>.
> +
> +"""Classes for working with JSON."""
> +
> +__metaclass__ = type
> +__all__ = ['DatetimeJSONEncoder']
> +
> +import datetime
> +import simplejson
> +
> +class DatetimeJSONEncoder(simplejson.JSONEncoder):
> + """A JSON encoder that understands datetime objects.
> +
> + Datetime objects are formatted according to ISO 1601.
> + """
> + def default(self, obj):
> + if isinstance(obj, datetime.datetime):
> + return obj.isoformat()
> + return simplejson.JSONEncoder.default(self, obj)
>
> === modified file 'launchpadlib/docs/modifications.txt'
> --- launchpadlib/docs/modifications.txt 2009-01-12 13:25:47 +0000
> +++ launchpadlib/docs/modifications.txt 2009-01-21 21:38:08 +0000
> @@ -305,3 +305,26 @@
> A conflicting display name
>
>
> +== Data types ==
> +
> +From the perspective of the launchpadlib user, date and date-time
> +fields always look like Python datetime objects.
> +
> + >>> firefox = launchpad.projects['firefox']
> + >>> milestone = firefox.all_milestones[0]
> + >>> milestone.date_targeted
> + datetime.datetime(2056, 10, 16,...)
> +
> +These fields can be changed and written back to the server, just like
> +objects of other types.
> +
> + >>> from datetime import datetime
> + >>> milestone.date_targeted = datetime(2009, 1, 1)
> + >>> milestone.lp_save()
> +
> +A datetime object may also be used as an argument to a named operation.
> +
> + >>> series = firefox.series[0]
> + >>> series.newMilestone(
> + ... name="test", date_targeted = datetime(2009, 1, 1))

niggle: kwargs usually don't have surrounding spaces (that is,
"date_targeted=datetime(2009, 1, 1)").

> + <milestone at ...>
>
> === modified file 'launchpadlib/docs/people.txt'
> --- launchpadlib/docs/people.txt 2008-10-02 16:08:22 +0000
> +++ launchpadlib/docs/people.txt 2009-01-21 21:38:08 +0000
> @@ -198,7 +198,7 @@
> >>> salgado.hide_email_addresses
> False
> >>> salgado.date_created
> - u'2005-06-06T08:59:51.596025+00:00'
> + datetime.datetime(2005, 6, 6, 8, 59, 51, 596025, ...)
> >>> print salgado.time_zone
> None
> >>> salgado.is_valid
> @@ -299,7 +299,7 @@
> >>> bassists.hide_email_addresses
> False
> >>> bassists.date_created
> - u'...'
> + datetime.datetime(...)
> >>> print bassists.time_zone
> None
> >>> bassists.is_valid
>
> === modified file 'launchpadlib/resource.py'
> --- launchpadlib/resource.py 2008-12-08 14:26:55 +0000
> +++ launchpadlib/resource.py 2009-01-21 21:38:08 +0000
> @@ -34,6 +34,7 @@
> from urlparse import urlparse
>
> from launchpadlib._utils.uri import URI
> +from launchpadlib._utils.json import DatetimeJSONEncoder
> from launchpadlib.errors import HTTPError
> from wadllib.application import Resource as WadlResource
>
> @@ -331,7 +332,7 @@
> http_method = self.wadl_method.name
> args = self._transform_resources_to_links(kwargs)
> for key, value in args.items():
> - args[key] = simplejson.dumps(value)
> + args[key] = simplejson.dumps(value, cls=DatetimeJSONEncoder)
> if http_method in ('get', 'head', 'delete'):
> url = self.wadl_method.build_request_url(**args)
> in_representation = ''
>
>

review: Approve (code)

« Back to merge proposal