Re: [Python-Dev] Minor ConfigParser Change
On 6/1/07, Fred L. Drake, Jr. [EMAIL PROTECTED] wrote: Changes in general are a source of risk; they have to be considered carefully. We've seen too many cases in which a change was thought to be safe, but broke something for someone. Avoiding style-only changes helps avoid introducing problems without being able to predict them; there are tests for ConfigParser, but it's hard to be sure every corner case has been covered. I understand what you mean, all changes carry a certain risk. Especially in code that is so widely relied upon as the Standard Library. But the alternative, which is to let the code rot, while one-line fixes are applied upon it, is a much worse alternative. It is true that unit tests does not cover all corner cases and that you can't be 100% sure that a change won't break something for someone. But on the other hand, the whole point with unit tests is to facilitate exactly these kind of changes. If something breaks then that is a great opportunity to introduce more tests. This is a general policy in the Python project, not simply my preference. I'd love to be able to say yes, the code is painful to read, let's make it nicer, but it's hard to say that without being able to say I'm sure it won't break anything for anybody. Python's too flexible for that to be easy. While what you have stated is the policy, I can't help but think that it is totally misguided (no offense intended). Maybe the policy can be reevaluated? -- mvh Björn ___ Python-Dev mailing list Python-Dev@python.org http://mail.python.org/mailman/listinfo/python-dev Unsubscribe: http://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com
Re: [Python-Dev] Minor ConfigParser Change
On Friday 01 June 2007, BJörn Lindqvist wrote: Patches are applied once, but thousands of people read the code in the standard library each month. The standard library should be as readable as possible to make it as easy as possible to maintain. It is just good software development methodology. Rest assured, I understand your sentiment here, and am not personally against an occaissional clean-up. ConfigParser in particular is old and highly idiosyncratic. Many parts of the standard library are arcane and almost impossible to understand (see httplib for example) because refactoring changes are Not done. So if someone wants to improve the code why not let them? Changes in general are a source of risk; they have to be considered carefully. We've seen too many cases in which a change was thought to be safe, but broke something for someone. Avoiding style-only changes helps avoid introducing problems without being able to predict them; there are tests for ConfigParser, but it's hard to be sure every corner case has been covered. This is a general policy in the Python project, not simply my preference. I'd love to be able to say yes, the code is painful to read, let's make it nicer, but it's hard to say that without being able to say I'm sure it won't break anything for anybody. Python's too flexible for that to be easy. -Fred -- Fred L. Drake, Jr. fdrake at acm.org ___ Python-Dev mailing list Python-Dev@python.org http://mail.python.org/mailman/listinfo/python-dev Unsubscribe: http://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com
Re: [Python-Dev] Minor ConfigParser Change
Fred, My only motivation was style. As per your comment: In general, we try to avoid making style changes to the code since that can increase the maintenance burden (patches can be harder to produce that can be cleanly applied to multiple versions). I will keep this in mind when supplying future patches. Joseph Armbruster On 5/31/07, Fred L. Drake, Jr. [EMAIL PROTECTED] wrote: On Saturday 26 May 2007, Joseph Armbruster wrote: I noticed that one of the parts of ConfigParser was not using for line in fp style of readline-ing :-) So, this will reduce the SLOC by 3 lines and improve readability. However, I did a quick grep and this type of practice appears in several other places. Before the current iteration support was part of Python, there was no way to iterate over a the way there is now; the code you've dug up is simply from before the current iteration support. (As I'm sure you know.) Is there motivation for these changes other than a stylistic preference for the newer idioms? Keeping the SLOC count down seems pretty minimal, and unimportant. Making the code more understandable is valuable, but it's not clear how much this really achieves that. In general, we try to avoid making style changes to the code since that can increase the maintenance burden (patches can be harder to produce that can be cleanly applied to multiple versions). No other motivat Are there motivations we're missing? -Fred -- Fred L. Drake, Jr. fdrake at acm.org ___ Python-Dev mailing list Python-Dev@python.org http://mail.python.org/mailman/listinfo/python-dev Unsubscribe: http://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com
Re: [Python-Dev] Minor ConfigParser Change
On Saturday 26 May 2007, Joseph Armbruster wrote: I noticed that one of the parts of ConfigParser was not using for line in fp style of readline-ing :-) So, this will reduce the SLOC by 3 lines and improve readability. However, I did a quick grep and this type of practice appears in several other places. Before the current iteration support was part of Python, there was no way to iterate over a the way there is now; the code you've dug up is simply from before the current iteration support. (As I'm sure you know.) Is there motivation for these changes other than a stylistic preference for the newer idioms? Keeping the SLOC count down seems pretty minimal, and unimportant. Making the code more understandable is valuable, but it's not clear how much this really achieves that. In general, we try to avoid making style changes to the code since that can increase the maintenance burden (patches can be harder to produce that can be cleanly applied to multiple versions). Are there motivations we're missing? -Fred -- Fred L. Drake, Jr. fdrake at acm.org ___ Python-Dev mailing list Python-Dev@python.org http://mail.python.org/mailman/listinfo/python-dev Unsubscribe: http://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com
Re: [Python-Dev] Minor ConfigParser Change
Kristján,
Here is a part of the patch that I was referring to. Something to that
effect.
Joseph Armbruster
Index: Lib/ConfigParser.py
===
--- Lib/ConfigParser.py (revision 55600)
+++ Lib/ConfigParser.py (working copy)
@@ -441,10 +441,7 @@
optname = None
lineno = 0
e = None # None, or an exception
-while True:
-line = fp.readline()
-if not line:
-break
+for line in fp:
lineno = lineno + 1
# comment or blank line?
if line.strip() == '' or line[0] in '#;':
Index: Lib/distutils/sysconfig.py
===
--- Lib/distutils/sysconfig.py (revision 55600)
+++ Lib/distutils/sysconfig.py (working copy)
@@ -216,10 +216,7 @@
define_rx = re.compile(#define ([A-Z][A-Za-z0-9_]+) (.*)\n)
undef_rx = re.compile(/[*] #undef ([A-Z][A-Za-z0-9_]+) [*]/\n)
#
-while 1:
-line = fp.readline()
-if not line:
-break
+for line in fp:
m = define_rx.match(line)
if m:
n, v = m.group(1, 2)
@@ -254,10 +251,7 @@
done = {}
notdone = {}
-while 1:
-line = fp.readline()
-if line is None:# eof
-break
+for line in fp:
m = _variable_rx.match(line)
if m:
n, v = m.group(1, 2)
Index: Lib/formatter.py
===
--- Lib/formatter.py(revision 55600)
+++ Lib/formatter.py(working copy)
@@ -432,10 +432,7 @@
fp = open(sys.argv[1])
else:
fp = sys.stdin
-while 1:
-line = fp.readline()
-if not line:
-break
+for line in fp:
if line == '\n':
f.end_paragraph(1)
else:
Index: Lib/ftplib.py
===
--- Lib/ftplib.py (revision 55600)
+++ Lib/ftplib.py (working copy)
@@ -435,9 +435,7 @@
'''Store a file in line mode.'''
self.voidcmd('TYPE A')
conn = self.transfercmd(cmd)
-while 1:
-buf = fp.readline()
-if not buf: break
+ for buff in fp:
if buf[-2:] != CRLF:
if buf[-1] in CRLF: buf = buf[:-1]
buf = buf + CRLF
Index: Lib/keyword.py
===
--- Lib/keyword.py (revision 55600)
+++ Lib/keyword.py (working copy)
@@ -62,9 +62,7 @@
fp = open(iptfile)
strprog = re.compile('([^]+)')
lines = []
-while 1:
-line = fp.readline()
-if not line: break
+for line in fp:
if '{1, ' in line:
match = strprog.search(line)
if match:
Index: Lib/mimetypes.py
===
--- Lib/mimetypes.py(revision 55600)
+++ Lib/mimetypes.py(working copy)
@@ -204,10 +204,7 @@
list of standard types, else to the list of non-standard
types.
-while 1:
-line = fp.readline()
-if not line:
-break
+for line in fp:
words = line.split()
for i in range(len(words)):
if words[i][0] == '#':
Index: Lib/urlparse.py
===
--- Lib/urlparse.py (revision 55600)
+++ Lib/urlparse.py (working copy)
@@ -353,9 +353,7 @@
except ImportError:
from StringIO import StringIO
fp = StringIO(test_input)
-while 1:
-line = fp.readline()
-if not line: break
+ for line in fp:
words = line.split()
if not words:
continue
Index: Tools/pynche/ColorDB.py
===
--- Tools/pynche/ColorDB.py (revision 55600)
+++ Tools/pynche/ColorDB.py (working copy)
@@ -50,10 +50,7 @@
self.__byname = {}
# all unique names (non-aliases). built-on demand
self.__allnames = None
-while 1:
-line = fp.readline()
-if not line:
-break
+ for line in fp:
# get this compiled regular expression from derived class
mo = self._re.match(line)
if not mo:
___
Python-Dev mailing list
Python-Dev@python.org
http://mail.python.org/mailman/listinfo/python-dev
Unsubscribe:
http://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com
Re: [Python-Dev] Minor ConfigParser Change
-Original Message- From: Joseph Armbruster [mailto:[EMAIL PROTECTED] I noticed that one of the parts of ConfigParser was not using for line in fp style of readline-ing I'm afraid my authority is limited to .c stuff having to do with pcbuild8, but I'm sure someone else here would like to comment. Kristján ___ Python-Dev mailing list Python-Dev@python.org http://mail.python.org/mailman/listinfo/python-dev Unsubscribe: http://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com
Re: [Python-Dev] Minor ConfigParser Change
Kristján, Whoops! My apologies. In any case, I created the bugs in the sf issue tracker for proper CM practice. Look under the patches section within sf.net. You should go ahead and close out the 2005 build ones then, once applied :-) Thank you again, Joseph Armbruster Kristján Valur Jónsson wrote: -Original Message- From: Joseph Armbruster [mailto:[EMAIL PROTECTED] I noticed that one of the parts of ConfigParser was not using for line in fp style of readline-ing I'm afraid my authority is limited to .c stuff having to do with pcbuild8, but I'm sure someone else here would like to comment. Kristján ___ Python-Dev mailing list Python-Dev@python.org http://mail.python.org/mailman/listinfo/python-dev Unsubscribe: http://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com
