Skip to content
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

Fixed #28076 -- Added support for PostgreSQL's interval format to parse_duration(). #8354

Merged
merged 1 commit into from Jul 4, 2017

Conversation

Projects
None yet
3 participants
@schinckel
Copy link
Contributor

commented Apr 13, 2017

@adamchainz adamchainz changed the title Allow handling postgres' interval format. Fixed #28076 -- Made date parsing handle Postgres' interval format Apr 13, 2017

@timgraham

This comment has been minimized.

Copy link
Member

commented Jun 1, 2017

Do you plan to continue this soon, Matthew?

@schinckel

This comment has been minimized.

Copy link
Contributor Author

commented Jun 1, 2017

I did see this PR yesterday when looking through for something else.

I can't remember why I stopped: I'll try to squeeze some time in this weekend.

@schinckel schinckel force-pushed the schinckel:issue/28076 branch from 8221578 to 9a27c96 Jun 11, 2017

@schinckel

This comment has been minimized.

Copy link
Contributor Author

commented Jun 11, 2017

Ah, it seems there was a little more to this than I had originally thought.

@@ -51,6 +51,18 @@
)


postrges_interval_re = re.compile(

This comment has been minimized.

Copy link
@schinckel

schinckel Jun 11, 2017

Author Contributor

This regex is very similar to the standard one, but I'm not sure if it can (or should) be merged with that one.

For instance, it's legal to have a - on any of the parts of a standard duration, but that doesn't happen on a postgres interval: you only get a maximum of two (one on the number of days, and one on the h:m:s.ms section).

This comment has been minimized.

Copy link
@adamchainz

adamchainz Jun 11, 2017

Member

s/postrges/postgres

This comment has been minimized.

Copy link
@schinckel

schinckel Jun 12, 2017

Author Contributor

Hahaha. I couldn't see this, even when writing the comment!

@@ -51,6 +51,18 @@
)


postgres_interval_re = re.compile(

This comment has been minimized.

Copy link
@timgraham

timgraham Jun 20, 2017

Member

Perhaps adding a comment of what this format looks like would help.

@@ -114,7 +126,8 @@ def parse_duration(value):
The preferred format for durations in Django is '%d %H:%M:%S.%f'.
Also supports ISO 8601 representation.
Also supports ISO 8601 representation,

This comment has been minimized.

Copy link
@timgraham

timgraham Jun 20, 2017

Member

"and PostgreSQL's interval format." (no comma)

This comment has been minimized.

Copy link
@schinckel

schinckel Jul 1, 2017

Author Contributor

Only the Day-Time interval format is supported, as the Year-Month and Mixed interval types cannot be represented as a datetime.timedelta() object.

@@ -128,3 +141,12 @@ def parse_duration(value):
kw['microseconds'] = '-' + kw['microseconds']
kw = {k: float(v) for k, v in kw.items() if v is not None}
return sign * datetime.timedelta(**kw)
match = postgres_interval_re.match(value)
if match:
kw = match.groupdict()

This comment has been minimized.

Copy link
@timgraham

timgraham Jun 20, 2017

Member

Did you consider trying to factor out the common code between the two if match branches? Not sure if that would help or hurt readability.

This comment has been minimized.

Copy link
@schinckel

schinckel Jul 1, 2017

Author Contributor

Okay, I have a version that seems to work.

@@ -80,6 +80,23 @@ def test_parse_python_format(self):
for delta in timedeltas:
self.assertEqual(parse_duration(format(delta)), delta)

def test_parse_postgres_format(self):
self.assertEqual(parse_duration('1 day'), timedelta(1))

This comment has been minimized.

Copy link
@timgraham

timgraham Jun 20, 2017

Member

I'd use:

test_values = (
    ('1 day', timedelta(1)),
    ....
)

along with a loop and self.subTest to avoid the need to copy/paste self.assertEqual(parse_duration so much.

This comment has been minimized.

Copy link
@schinckel

schinckel Jul 1, 2017

Author Contributor

Oh, nice. First time I've had a chance to use subTest. I'll create a PR for making the same change in another test in this same function.

@timgraham timgraham changed the title Fixed #28076 -- Made date parsing handle Postgres' interval format Fixed #28076 -- Added support for PostgreSQL's interval format to parse_duration(). Jun 20, 2017

@django django deleted a comment from adamchainz Jun 20, 2017

@schinckel schinckel force-pushed the schinckel:issue/28076 branch from d444e39 to 19d4383 Jul 1, 2017

@timgraham timgraham force-pushed the schinckel:issue/28076 branch from 19d4383 to 1d36fcf Jul 3, 2017

@timgraham timgraham force-pushed the schinckel:issue/28076 branch from 1d36fcf to 493f7e9 Jul 3, 2017

@timgraham timgraham merged commit 493f7e9 into django:master Jul 4, 2017

17 checks passed

docs Build #17625 ended
Details
flake8 Build #17739 ended
Details
isort Build #17752 succeeded in 18 sec
Details
pull-requests-javascript Build #14118 ended
Details
pull-requests-trusty/database=mysql,label=trusty-pr,python=python3.4 Build #13389 ended
Details
pull-requests-trusty/database=mysql,label=trusty-pr,python=python3.6 Build #13389 ended
Details
pull-requests-trusty/database=mysql_gis,label=trusty-pr,python=python3.4 Build #13389 ended
Details
pull-requests-trusty/database=mysql_gis,label=trusty-pr,python=python3.6 Build #13389 ended
Details
pull-requests-trusty/database=postgis,label=trusty-pr,python=python3.4 Build #13389 ended
Details
pull-requests-trusty/database=postgis,label=trusty-pr,python=python3.6 Build #13389 ended
Details
pull-requests-trusty/database=postgres,label=trusty-pr,python=python3.4 Build #13389 ended
Details
pull-requests-trusty/database=postgres,label=trusty-pr,python=python3.6 Build #13389 ended
Details
pull-requests-trusty/database=spatialite,label=trusty-pr,python=python3.4 Build #13389 ended
Details
pull-requests-trusty/database=spatialite,label=trusty-pr,python=python3.6 Build #13389 ended
Details
pull-requests-trusty/database=sqlite3,label=trusty-pr,python=python3.4 Build #13389 ended
Details
pull-requests-trusty/database=sqlite3,label=trusty-pr,python=python3.6 Build #13389 ended
Details
pull-requests-windows/database=sqlite3,label=windows,python=Python35 Build #9700 ended
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.