Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Iso8601 truncation #79

Merged
merged 4 commits into from May 6, 2013

Conversation

Projects
None yet
3 participants
Contributor

lmctv commented Jan 29, 2013

The encosed changes are needed for correct handling of android chrome's native datetime
form input, which is truncated to the minute. While at it, I added support for truncations up to the day level.

Contributor's agreement merged in from a separate branch (to avoid timing problems between this request and #78).

Thank you,

  l.

lmctv added some commits Jan 29, 2013

Iso datetime format can be truncated.
Correct ISO8601_REGEX to support truncated time part in the
datetime representation, and add corresponding tests: now we support
time stamps truncated to the minute, the hour or the day:

    2007-01-25T12:00Z
    2007-01-25T12Z
    2007-01-25

@lmctv lmctv referenced this pull request in Pylons/deform Jan 29, 2013

Merged

Add support for html5 native time-related widgets #142

Owner

tseaver commented Feb 6, 2013

The changes here look good to me.

Contributor

douglatornell commented Mar 18, 2013

Confirmed that dropping lower-order components of time is in accordance with standard (ISO 8601:2004 clause 4.2.2.3 Representations with reduced accuracy).

Patch applies cleanly, but results in test coverage < 100%. Investigating...

Contributor

lmctv commented Mar 19, 2013

@douglatornell The coverage-excluded lines are inside Date[Time]'s deserialize method; they are missed since the truncated
format no longer raises a iso8601.ParseError when called from both TestDate.test_deserialize_success_date
and TestDateTime.test_deserialize_date.

I've just removed the back-tracking date build.

Thank you very much!

Contributor

douglatornell commented Mar 21, 2013

@lmctv I agree that 5214a22 makes it so that DateTime.deserialize() no longer raises iso8601.ParseError for truncated date/time strings. In fact, I've convinced myself and @mcdonc that iso8601.parse_date() can only raise a ParseError and that catching a TypeError is no longer required in DateTime.deserialize(), so line 1409 can become:

except iso8601.ParseError as e:

There's a similar pattern in Date.deserialize() but I'm still working on understanding why 1480 is not covered there with 5214a22 applied.

Contributor

lmctv commented Mar 21, 2013

Before 5214a22, Date.deserialize was unconditionally falling in the exception: clause since
TestDate.test_deserialize_success_date() builds a ISO formatted date.

I just simplified the exception: clauses by catching only iso8601.ParseError.

A nosetest --with-cover shows full coverage both at fbae200 and at 89197d3:

   Name                           Stmts   Miss  Cover   Missing
   ------------------------------------------------------------
   colander                         905      0   100%   
    ...

Thnak you very much @douglatornell

@tseaver tseaver merged commit 89197d3 into Pylons:master May 6, 2013

Owner

tseaver commented May 6, 2013

Thanks @lmctv and @douglatornell !

Contributor

lmctv commented May 6, 2013

Great! Thank you very much @tseaver and @douglatornell

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment