All,
As per the removal of the rgbimgmodule due to deprecation in the trunk:
http://svn.python.org/projects/python/trunk
---Begin SVN log---
r55458 | brett.cannon | 2007-05-20 03:09:50 -0400 (Sun, 20 May 2007) | 2
lines
Remove the rgbimg module. It has been deprecated since Python 2.5.
---End SVN log---
The PCbuild8 solution needs to be corrected. A patch is attached.
In addition, I noticed that under C++/Advanced Properties, all the
configurations appear to be set to "Compile as C++ Code" with the /TP
argument. Should these be set to "Compile as C Code" with the /TC argument?
Joseph Armbruster
Index: PCbuild8/pythoncore/pythoncore.vcproj
===================================================================
--- PCbuild8/pythoncore/pythoncore.vcproj (revision 55600)
+++ PCbuild8/pythoncore/pythoncore.vcproj (working copy)
@@ -1518,10 +1518,6 @@
>
</File>
-----Original Message----- From: python-dev-bounces+kristjan=ccpgames.com@python.org The PCbuild8 solution needs to be corrected. A patch is attached.
Thanks, I'll apply it.
In addition, I noticed that under C++/Advanced Properties, all the configurations appear to be set to "Compile as C++ Code" with the /TP argument. Should these be set to "Compile as C Code" with the /TC argument?
Interesting. I hadn't noticed. I investigated, and this is the default value for all projects. However, if you click a single .c file and check its properties, you will find that it gets the /TC flag in its advanced settings. So each file will be correctly compiled. (you can confirm this by checking the command line). Removing the /TP flag from the project settings also results in the disappearance of the per-file /TC setting. Very curious. In end effect, the C files are compiled as such and there is no need for panic. Cheers, Kristján
Kristján, I started to investigate the WINVER warnings that are scattered throughout the VS 2005 build. This patch eliminates them but I may have overlooked the intentions of the #include ordering. If this invalid, please let me know. Patch attached. Joseph Armbruster Kristján Valur Jónsson wrote:
-----Original Message----- From: python-dev-bounces+kristjan=ccpgames.com@python.org The PCbuild8 solution needs to be corrected. A patch is attached.
Thanks, I'll apply it.
In addition, I noticed that under C++/Advanced Properties, all the configurations appear to be set to "Compile as C++ Code" with the /TP argument. Should these be set to "Compile as C Code" with the /TC argument?
Interesting. I hadn't noticed. I investigated, and this is the default value for all projects. However, if you click a single .c file and check its properties, you will find that it gets the /TC flag in its advanced settings. So each file will be correctly compiled. (you can confirm this by checking the command line). Removing the /TP flag from the project settings also results in the disappearance of the per-file /TC setting. Very curious. In end effect, the C files are compiled as such and there is no need for panic.
Cheers, Kristján
Index: PC/_winreg.c
===================================================================
--- PC/_winreg.c (revision 55600)
+++ PC/_winreg.c (working copy)
@@ -11,9 +11,8 @@
basic Unicode support added.
*/
-
-#include "windows.h"
#include "Python.h"
+#include "windows.h"
#include "structmember.h"
#include "malloc.h" /* for alloca */
Index: PC/dl_nt.c
===================================================================
--- PC/dl_nt.c (revision 55600)
+++ PC/dl_nt.c (working copy)
@@ -7,12 +7,12 @@
forgotten) from the programmer.
*/
-#include "windows.h"
+
/* NT and Python share these */
#include "pyconfig.h"
#include "Python.h"
-
+#include "windows.h"
char dllVersionBuffer[16] = ""; // a private buffer
// Python Globals
Index: Python/dynload_win.c
===================================================================
--- Python/dynload_win.c (revision 55600)
+++ Python/dynload_win.c (working copy)
@@ -1,13 +1,12 @@
/* Support for dynamic loading of extension modules */
-
+#include "Python.h"
#include
Kristján, While we are on the topic of minor changes... :-) 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. There is a possibility of good savings in this department. If you think this is worthwhile, I can create one large patch for them all. I sure hope I am not missing something fundamental with this one... 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 '#;':
-----Original Message----- From: Joseph Armbruster [mailto:josepharmbruster@gmail.com]
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
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:josepharmbruster@gmail.com]
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
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>
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.
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>
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. 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? -- mvh Björn
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>
On 6/1/07, Fred L. Drake, Jr.
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
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:
participants (4)
-
BJörn Lindqvist -
Fred L. Drake, Jr. -
Joseph Armbruster -
Kristján Valur Jónsson