Guido van Rossum added the comment: Couple of nits:
- You added a removal of hotshot from setup.py to the patch; but that's been checked in in the mean time. - Why add an 'errors' argument to the function when it's a fatal error to use it? - Using 0 to autodetect the length is scary. Normally we have two APIs for that, one ..._FromString and one ...FromStringAndSize. If you really don't want that, please use -1, which is at least an illegal value. - Why is there code in codeobject.c::PyCode_New() that still accepts a PyString for the filename? - In that file (and possibly others, I didn't check) your code uses spaces to indent while the surrounding code uses tabs. Moreover, your space indent seems to assume there are 4 spaces to a tab, but all our code (Python and C) is formatted assuming tabs are 8 spaces. (The indent isn't always 8 spaces -- but ASCII TAB characters always are 8, for us.) - Why copy the default encoding before mangling it? With a little extra care you will only have to copy it once. Also, consider not mangling at all, but assuming the encoding comes in a canonical form -- several other functions assume that, e.g. PyUnicode_Decode() and PyUnicode_AsEncodedString(). - I haven't run the unit tests yet. Will be doing that next... __________________________________ Tracker <[EMAIL PROTECTED]> <http://bugs.python.org/issue1272> __________________________________ _______________________________________________ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
