[Patches] [ python-Patches-593560 ] bugfixes and cleanup for _strptime.py

noreply@sourceforge.net noreply@sourceforge.net
Tue, 03 Sep 2002 11:21:22 -0700


Patches item #593560, was opened at 2002-08-10 19:01
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=305470&aid=593560&group_id=5470

Category: Library (Lib)
Group: Python 2.3
Status: Open
Resolution: None
Priority: 5
Submitted By: Brett Cannon (bcannon)
Assigned to: Barry A. Warsaw (bwarsaw)
Summary: bugfixes and cleanup for _strptime.py

Initial Comment:
Discovered two bugs in _strptime.py thanks to Mikael
Sch?berg of AB Strakt; both were in
LocaleTime.__calc_date_time().  One was where if a
locale-specific format string represented the month
without a leading zero, it would not be caught.  The
other bug was when a locale just lacked some
information (in this case, Swedish's lack of an AM/PM
representation); IndexError was thrown because
string.replace() was being called with the empty string
as the old value.

I also took this opportunity to clean up some of the
code (namely TimeRE.__getitem__() along with
LocaleTime.__calc_date_time()).  Added some comments,
reformatted some code, etc.  All of this was brought on
thanks to the Python Cookbook's chapter 1 (good work
Alex and David!).

I have updated test_strptime.py to check for the second
of the mentioned bug explicitly.  I also commented the
code and added a fxn that creates a PyUnit test suite
with all of the tests.

----------------------------------------------------------------------

>Comment By: Brett Cannon (bcannon)
Date: 2002-09-03 11:21

Message:
Logged In: YES 
user_id=357491

Yes, the current diffs do not have a fix for the problem you 
emailed me about.  They are also do not correspond to the 
newest CVS checkin that you did a few days back.  I will 
make separate patches for this newest bug (I think it is 
just a test_strptime.py bug), the case insensitivity fix, and 
my code cleanup/doc string cleanup.  I don't have an ETA 
on this, though.

----------------------------------------------------------------------

Comment By: Barry A. Warsaw (bwarsaw)
Date: 2002-09-03 11:02

Message:
Logged In: YES 
user_id=12800

Brett, I think the diff that's currently uploaded here is
not your latest one, which you mention in your 2002-08-30
15:26 follow up.  Can you see if you can fix the
test_date_time() failure that I emailed you privately today
also?

To recap for others reading this report, if the day of the
month < 10 (as it is today 3-Sep-2002), %d writes the
day-of-month field as "03" while %c writes it as " 3" in my
locale (C).  I don't know whether there are any guarantees
about what %c will return, so it's possible that the test is
just bogus.  I'm not sure what the intent of the test is, so
I've asked Brett in a private message for clarification or a
patch.

This is assigned to me, so I'll vette Brett's patch when he
uploads the newest version.

----------------------------------------------------------------------

Comment By: Guido van Rossum (gvanrossum)
Date: 2002-09-03 10:52

Message:
Logged In: YES 
user_id=6380

Assigning to Barry so he can check this in.

----------------------------------------------------------------------

Comment By: Brett Cannon (bcannon)
Date: 2002-08-30 12:26

Message:
Logged In: YES 
user_id=357491

Yeah, I noticed the email on Python-dev.  His patch 
actually overlaps mine since I did pretty much the same 
stylized changes in this patch (although I didn't have the 
AM/PM fix; wonder why that suddenly popped up?).  At 
least I don't have to deal with that now.

Since I am going to have to do a whole new diff I will try to 
separate the bugfix patches from the code style/doc string 
benign changes.

And thanks for the CVS help.  Hopefully I will be able to 
figure it out; if not, expect an email.  =)

----------------------------------------------------------------------

Comment By: Neal Norwitz (nnorwitz)
Date: 2002-08-29 15:18

Message:
Logged In: YES 
user_id=33168

Brett, unfortunately Barry just checked in some changes to
strptime, so this is out of date.  (He fixed an am/pm
problem.)  

Could you generate new patches?  Also, could you separate
into separate patch files the bug fix/semantic changes from
docstring changes, whitespace, etc.  Typically, unrelated
changes are checked in separately.

To produce a diff from CVS, do:  cvs diff -C 5 > patch.diff
If you need CVS help, you can mail me directly.

----------------------------------------------------------------------

Comment By: Brett Cannon (bcannon)
Date: 2002-08-26 13:16

Message:
Logged In: YES 
user_id=357491

So I was right and now have submitted a patch for PyUnit on
its SF patch DB to fix the problem.  And now that I fixed it
I can say confidently that the changes pass the testing suite.

I have attached two contextual diffs below (couldn't get cvs
diff to work; still learning CVS); one diff is for
_strptime.py and the other is for test/test_strptime.py.

----------------------------------------------------------------------

Comment By: Brett Cannon (bcannon)
Date: 2002-08-24 14:08

Message:
Logged In: YES 
user_id=357491

So I have a patch for both _strptime.py and test_strptime.py
for Jason's bug, but I want to be able to run it through
PyUnit first.  For some strange reason, PyUnit will not
auto-execute my tests under my Python 2.3 install
(unittest.main()).  Hopefully the latest CVS will deal with
it.  If not, I might have a patch for PyUnit.  =)

Either way, I will have the files up, at the latest, at the
end of the month even if it means I have to execute the
tests at the interpreter.

Oh, and I will do a CVS diff (or diff -c) this time for the
patches instead of posting the whole files.

----------------------------------------------------------------------

Comment By: Brett Cannon (bcannon)
Date: 2002-08-23 20:16

Message:
Logged In: YES 
user_id=357491

A bug issue was brought up in patch 474274 which was the
original _strptime patch.  I will address it and append it
to this patch.

----------------------------------------------------------------------

Comment By: Brett Cannon (bcannon)
Date: 2002-08-14 02:07

Message:
Logged In: YES 
user_id=357491

Just as a follow-up, I got an email from Mikael on Mon.,
2002-08-12, letting me know that the patch seems to have
worked for the bug he discovered.

----------------------------------------------------------------------

Comment By: Brett Cannon (bcannon)
Date: 2002-08-11 15:16

Message:
Logged In: YES 
user_id=357491

Sorry, Martin.  I thought I remembered reading somewhere
that for Python files you can just post the whole thing.  I
will stop doing that.

As for Mikael and the patch, he says that it appears to be
working.  I gave it to him on Tuesday and he said it
appeared to be working; he has yet to say otherwise.  If you
prefer, I can have him post here to verify this.

----------------------------------------------------------------------

Comment By: Martin v. Löwis (loewis)
Date: 2002-08-11 10:47

Message:
Logged In: YES 
user_id=21627

Please don't post complete files. Instead, post context (-c)
or unified (-u) diffs. Ideally, produce them with "cvs
diff", as this will result in patches that record the CVS
version number they were for.

I think it would be good to get a comment from Mikael on
that patch.

----------------------------------------------------------------------

Comment By: Brett Cannon (bcannon)
Date: 2002-08-10 20:31

Message:
Logged In: YES 
user_id=357491

Just when you thought you had something done, tim_one had to
go and normalize the whitespace in both _strptime.py and
test_strptime.py!  =)

So to save Tim the time and effort of having to normalize
the files again, I went ahead and applied them to the fixed
files.  I also reformatted test_strptime.py so that lines
wrapped around 80 characters (didn't realize Guido had added
it to the distro until today).

So make sure to use the files that specify whitespace
normalization in their descriptions.

----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=305470&aid=593560&group_id=5470