[ZCM] [ZC] 2191/16 Comment "DateTime doesn't parse all
ISO8601-valid strings"
Collector: Zope Bugs, Features,
and Patches ...
zope-coders-admin at zope.org
Tue Feb 27 13:40:26 EST 2007
Issue #2191 Update (Comment) "DateTime doesn't parse all ISO8601-valid strings"
Status Accepted, Zope/bug medium
To followup, visit:
http://www.zope.org/Collectors/Zope/2191
==============================================================
= Comment - Entry #16 by eikenberry on Feb 27, 2007 1:40 pm
Looks like it is mistakenly parsing the '-' dates as iso8601.
The big iso8601Match regex matches the first example below, which it shouldn't (due to the space).
Eg.
>>> DateTime('2007-02-27 13:09:01.279').timezone()
'GMT+0'
>>> DateTime('2007/02/27 13:09:01.279').timezone()
'US/Eastern'
________________________________________
= Comment - Entry #15 by bacarter on Feb 26, 2007 9:18 pm
This patch appears to break backwards API compatibility. DateTime() objects now assume any date entered is in GMT unless a timezone is specified. This is contrary to zope 2.9.5 behavior where the timezone is assumed to be the system default. The new code is also inconsistent:
print "\n>>>>>> ", DateTime().timezone()
print "\n>>>>>> ", DateTime('2006-02-27').timezone()
print "\n>>>>>> ", DateTime('2006-02-27 12:30:00').timezone()
print "\n>>>>>> ", DateTime().greaterThan(DateTime('2007-02-27 01:00:00'))
Prints:
>>>>>> US/Pacific
>>>>>> GMT+0
>>>>>> GMT+0
>>>>>> True
Note that a DateTime() with no string returns an instance in the the machine's timezone (Pacific) however all other instances are in GMT. This really screws up the final comparison, where we end up comparing a GMT time to a Pacific time.
________________________________________
= Comment - Entry #14 by tseaver on Dec 13, 2006 12:51 pm
> = Comment - Entry #13 by dtremea on Dec 13, 2006 12:19 pm
> sorry for not have fixed this on the last weekend as I promised, but my
> goal is to do it later today. In fact, I have 80% from it done, which
> means we add the ISO8601 support and *keep* the backward compatibility.
I'm sorry if my reversions today on the 2.9 and 2.10 branchs
make your merge harder. :(
> Some remarks: about the docstrings on tests, yep, I'm aware of it,
> that's why I changed them to comments (and not the other way round).
My bad -- I think I was looking at a reversed diff.
> About the cosmetical code changes, sorry for that, but I just
> standardized with the remaining code from the same file... :-)
I will assume that was my bad, too.
In general, I would avoid committing "cleanup" changes like those
at the same time as "substantive" changes, and would only commit such "cleanups" on the trunk.
________________________________________
= Comment - Entry #13 by dtremea on Dec 13, 2006 12:19 pm
Hey Tres,
sorry for not have fixed this on the last weekend as I promised, but my goal is to do it later today. In fact, I have 80% from it done, which means we add the ISO8601 support and *keep* the backward compatibility.
Some remarks: about the docstrings on tests, yep, I'm aware of it,
that's why I changed them to comments (and not the other way round).
About the cosmetical code changes, sorry for that, but I just standardized with the remaining code from the same file... :-)
________________________________________
= Comment - Entry #12 by tseaver on Dec 13, 2006 11:35 am
There is also a bunch of "gratuitous" janitorial change (e.g., removing periods at the end of doctstring sentences) which is
both debatable on the merits (An imperative sentence *ought* to
end with a period) and inappropriate for code long in maintenance
mode (it makes finding the "real" changes, or applying other
patches, needlessly difficult.
________________________________________
= Comment - Entry #11 by tseaver on Dec 13, 2006 11:32 am
Note as well that docstrings are not allowed for testcase methods:
the testrunner displays them, if present, rather than the name
of the method, which is not acceptable.
________________________________________
= Comment - Entry #10 by tseaver on Dec 13, 2006 11:31 am
This change has to be reverted on the release branches, as it
breaks semantics of peoples *persistent data* with no possibility
of correct BBB.
On the trunk, such a change should come with an *option" to enable
the new semantics; that option should be *disabled* through
at least the next release, at which point it might safely be
enabled.
I will revert on the 2.9 and 2.10 branches.
________________________________________
= Resubmit - Entry #9 by shh on Nov 23, 2006 5:03 am
Status: Resolved => Accepted
FWIW, I am all for backward compatiblity in this case.
"Naive" ISO (w/o timezone) used to mean local time. I'd be very reluctant to change this behavior, even though I agree it is "wrong". There are currently tests breaking in CMF and ATContentTypes because of this, and I am worried that there is a lot of code out there relying on the local time interpretation.
I could however imagine a zope.conf flag, say, 'datetime-strict-iso-parsing' that would be *off* by default, at least on the maintenance branches (2.9, 2.10).
________________________________________
= Comment - Entry #8 by dtremea on Nov 22, 2006 11:26 am
Looks like this fix broke 3 CMF tests:
http://mail.zope.org/pipermail/cmf-tests/2006-November/003324.html
After further investigation, I would say that "YYYY-MM-DD" dates
are in the ISO8601 format, and thus GMT based.
Dates in format "YYYY-MM-DD HH:MM:SS" are also in ISO format, but
Zope wasn't recognizing they as such. Now Zope does.
I argue that CMF was misusing DateTime, and that the new behavior
is the correct, otherwise we'll never get a consistent behavior
for the ISO dates.
I argue that the specific getBeginAndEndTimes method from
CalendarTool.py should also use the timezone, analog to what the
getEventsForThisDay method does.
Thoughts?
________________________________________
= Resolve - Entry #7 by dtremea on Nov 20, 2006 10:02 pm
Status: Accepted => Resolved
Fixed on:
- 2.9: http://svn.zope.org/Zope/branches/2.9/?rev=71230&view=rev
- 2.10: http://svn.zope.org/Zope/branches/2.10/?rev=71231&view=rev
- trunk: http://svn.zope.org/Zope/trunk/?rev=71232&view=rev
________________________________________
= Comment - Entry #6 by tuppence on Nov 7, 2006 5:52 am
I don't have time to test this new patch, but it sounds like a *considerable* improvement to mine. If others are happy, so am I!
Thanks Dorneles :).
________________________________________
= Accept - Entry #5 by dtremea on Nov 5, 2006 4:00 pm
Status: Pending => Accepted
Supporters added: dtremea
Uploaded: "DateTime-ISO8601.patch"
- http://www.zope.org/Collectors/Zope/2191/DateTime-ISO8601.patch/view
Hey folks,
> The patch is supposed to support both formats, and indeed, it
> does seem to. The test also tests both formats.
>
> The problem is that another, apparenly unrelated test, now fails
> (with details below). I'm really not sure why this separate test
> fails now. If anyone can suggest anything to try after looking
> at the failure below, I'd love to hear it.
I read the ISO8601 3rd edition standard specification[1], from 2004-12-01, and added support for these features:
- reduce date format
- week format
- day of year format
- hours/minutes fractions
- optional date/time separators
- optional seconds/minutes
- timezones with only one digit for hour
Attached is the patch with tests for each feature. Additionally,
it fixes the failing test introduced by the initial work on this
issue.
Please review it and give me a '+1', so I can commit my first
Zope code contribution!
Any feedback is welcome.
[1] http://www.omg.org/docs/ISO-stds/06-08-01.pdf
PS: Don't be afraid of that 45 lines regex, it's just for the
sake of readability and understanding... :-)
________________________________________
= Comment - Entry #4 by tuppence on Oct 15, 2006 7:55 am
The patch is supposed to support both formats, and indeed, it does seem to. The test also tests both formats.
The problem is that another, apparenly unrelated test, now fails (with details below). I'm really not sure why this separate test fails now. If anyone can suggest anything to try after looking at the failure below, I'd love to hear it.
"""
tim at teresa 2.8.8 $ bin/zopectl test --libdir lib/python/DateTime
Running tests via: /usr/bin/python /home/tim/zope/freethink/2.8.8/bin/test.py -v --config-file /home/tim/zope/freethink/2.8.8/etc/zope.conf --libdir lib/python/DateTime
Running unit tests at level 1
Running unit tests from /home/tim/zope/freethink/2.8.8/lib/python/DateTime
Parsing /home/tim/zope/freethink/2.8.8/etc/zope.conf
..............F.................
======================================================================
FAIL: testInternationalDateformat (tests.testDateTime.DateTimeTests)
----------------------------------------------------------------------
Traceback (most recent call last):
File "/home/tim/zope/freethink/2.8.8/lib/python/DateTime/tests/testDateTime.py", line 343, in testInternationalDateformat
self.assertEqual(d_us, d_int)
File "/usr/lib/python2.3/unittest.py", line 302, in failUnlessEqual
raise self.failureException, \
AssertionError: DateTime('1990/01/02') != DateTime('1990/01/01')
----------------------------------------------------------------------
Ran 32 tests in 30.515s
FAILED (failures=1)
"""
________________________________________
= Comment - Entry #3 by ajung on Oct 15, 2006 2:09 am
ISO8601 allows both YYYY-MM-DD and YYYYMMDD. DateTime should support *both*. A related patch/unittest must support both.
________________________________________
= Comment - Entry #2 by tuppence on Sep 12, 2006 6:59 pm
Uploaded: "testdatetime.diff"
- http://www.zope.org/Collectors/Zope/2191/testdatetime.diff/view
And here is a patch that tests the new behaviour.
The problem is that the first patch uploaded here causes another test to fail. I cannot see how or why, and probing with DateTime manually from the python prompt, I'm unable to reproduce the test failure.
If anyone with a better grasp of this has any ideas, I'd love to hear them.
________________________________________
= Request - Entry #1 by tuppence on Sep 12, 2006 6:57 pm
Uploaded: "datetime.diff"
- http://www.zope.org/Collectors/Zope/2191/datetime.diff/view
The DateTime.DateTime constructor successfully parses ISO8601 strings like '2006-09-12T20:33:11Z' but does not recognise them like '20060912T20:33:11Z'. According to http://www.w3.org/TR/NOTE-datetime, the latter is not valid, but http://www.cl.cam.ac.uk/~mgk25/iso-time.html suggests otherwise. I'm not sure which is correct, but I have a problem in that Ecto (the remote blogging tool) is sending strings like the latter and I need to parse them.
I'm uploading a diff that enables the behaviour that I'm after.
==============================================================
More information about the Zope-Collector-Monitor
mailing list