Author lemburg
Recipients belopolsky, eric.smith, ezio.melotti, lemburg, mark.dickinson, skrah, vstinner
Date 2010-12-03.09:45:01
SpamBayes Score 3.88578e-15
Marked as misclassified No
Message-id <4CF8BC1C.8070803@egenix.com>
In-reply-to <AANLkTimx9h8SE2-DPSikyR7AJECuPmyJ1douXeRt0AqP@mail.gmail.com>
Content
Alexander Belopolsky wrote:
> 
> Alexander Belopolsky <belopolsky@users.sourceforge.net> added the comment:
> 
> On Thu, Dec 2, 2010 at 4:34 PM, Marc-Andre Lemburg
> <report@bugs.python.org> wrote:
> ..
>>  * Please change the API _PyUnicode_NormalizeDecimal() to
>>   PyUnicode_ConvertToASCIIDecimal() - that's closer to what
>>   it does.
>>
> 
> Are you sure it is a good idea to give it a public name?  I have no
> problem with calling it _PyUnicode_ConvertToASCIIDecimal().
> ("Transform" may be a better term, though.)

Yes, I think it's useful to have as public API.

>>  * Don't have the API remove any whitespace. It should just
>>   work on decimal digit code points (chainging the length
>>   of the Unicode string is a bad idea).
>>
> 
> Yes, that was a bad idea, but the old EncodeDecimal was replacing all
> Unicode space with ASCII ' '.  It will be hard to replicate old
> behavior without doing the same in  ConvertToASCIIDecimal().

Are you sure ? I'm not sure how the underlying PyOS_string_to_double()
(IIRC) works. If it needs ASCII spaces, it's probably OK to just do
the replacement in the constructors. This may have a side-effect
on error reporting, though, e.g. if you convert TABs to single
spaces, the error message will use different formatting than
the original string.

>>  * Please remove the note "This function is no longer used.
>>   Use _PyUnicode_NormalizeDecimal instead." from the
>>   PyUnicode_EncodeDecimal() API description in the
>>   header file. The API won't go away (it does have its
>>   use and is being used in 3rd party extensions) and
>>   you cannot guide people to use a Python private API.
>>
> 
> OK.  I had the same reservations about recommending private API.
> 
>>  * Please double check the ref counts. I think you have a leak
>>   in PyLong_FromUnicode() (for norm) and possible in other
>>   functions as well.
> 
> Will do.  I should also add some more tests for error conditions.  I
> test for leaks, but if the error branch is not covered, it is not
> covered.

Thanks. Error branches often introduce such leaks; which is why
I got used to having an onError: goto target in functions which
then do Py_XDECREF() on all temporary variables I need in the
function.
History
Date User Action Args
2010-12-03 09:45:03lemburgsetrecipients: + lemburg, mark.dickinson, belopolsky, vstinner, eric.smith, ezio.melotti, skrah
2010-12-03 09:45:02lemburglinkissue10557 messages
2010-12-03 09:45:01lemburgcreate