[issue20260] Argument Clinic: add unsigned integers converters

[Serhiy Storchaka]

Change by Serhiy Storchaka :


--
resolution:  -> fixed
stage: patch review -> resolved
status: open -> closed
versions: +Python 3.8 -Python 3.4

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue20260] Argument Clinic: add unsigned integers converters

[Serhiy Storchaka]

Serhiy Storchaka  added the comment:


New changeset 7cb7bcff20a386bba59cbc51e2419542de358bd2 by Serhiy Storchaka in 
branch 'master':
bpo-20260: Implement non-bitwise unsigned int converters for Argument Clinic. 
(GH-8434)
https://github.com/python/cpython/commit/7cb7bcff20a386bba59cbc51e2419542de358bd2


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue20260] Argument Clinic: add unsigned integers converters

[Serhiy Storchaka]

Change by Serhiy Storchaka :


--
pull_requests: +7959

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue20260] Argument Clinic: add unsigned integers converters

[Serhiy Storchaka]

Changes by Serhiy Storchaka storch...@gmail.com:


--
components: +Argument Clinic

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue20260
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue20260] Argument Clinic: add unsigned integers converters

[Stefan Krah]

Changes by Stefan Krah stefan-use...@bytereef.org:


--
nosy:  -skrah

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue20260
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue20260] Argument Clinic: add unsigned integers converters

[Josh Rosenberg]

Changes by Josh Rosenberg shadowranger+pyt...@gmail.com:


--
nosy: +josh.rosenberg

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue20260
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue20260] Argument Clinic: add unsigned integers converters

[Larry Hastings]

Larry Hastings added the comment:

I can start citing data points if you like.

socket.if_indextoname just calls PyLong_AsUnsignedLong().  Not sure what that 
throws.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue20260
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue20260] Argument Clinic: add unsigned integers converters

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

I think that we do not yet have enough data. Too little cases are known for 
which unsigned integer converters are needed, and different cases require a 
different behavior. I prefer to defer until we convert the majority of the 
code. Then we will see what behavior is most prevalent. We even can change and 
unify behavior in 3.5.

There is no hurry. We can use in each module its own Converter (if needed). 
There should not be many such places.

--
priority: normal - low

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue20260
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue20260] Argument Clinic: add unsigned integers converters

[Larry Hastings]

Larry Hastings added the comment:

Is this waiting on something?  I agree that we can't change the behavior
of existing functions.  But your new converters should raise ValueError.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue20260
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue20260] Argument Clinic: add unsigned integers converters

[Larry Hastings]

Larry Hastings added the comment:

Also, you didn't remove the _ in front of Converter in the names, e.g 
_PyLong_UnsignedShort_Converter should be _PyLong_UnsignedShortConverter.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue20260
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue20260] Argument Clinic: add unsigned integers converters

[Larry Hastings]

Larry Hastings added the comment:

Actually, here's another data point to consider.  The underlying implementation 
of PyArg_Parse* is the function convertsimple() in Python/getargs.c.  For all 
the non-bitwise integer format units ('bhi'), if they overflow or underflow the 
native integer they raise OverflowError.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue20260
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue20260] Argument Clinic: add unsigned integers converters

[Ryan Smith-Roberts]

Ryan Smith-Roberts added the comment:

socketmodule has three builtins which use PyLong_AsUnsignedLong on their 
arguments and would benefit from these converters, but only if they raise 
OverflowError. So I vote for #2.

I don't think it's unreasonable to continue to have locally-defined argument 
converters.

--
nosy: +rmsr

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue20260
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue20260] Argument Clinic: add unsigned integers converters

[Larry Hastings]

Larry Hastings added the comment:

We can use this now.  So I bumped it to the top of my queue and reviewed it.  I 
had only minor feedback--thanks!

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue20260
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue20260] Argument Clinic: add unsigned integers converters

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

Unfortunately I have discovered that there is significant difference between 
uint_converter in Modules/zlibmodule.c and proposed 
_PyLong_UnsignedInt_Converter. And there are tests in Lib/test/test_zlib.py 
which fail when _PyLong_UnsignedInt_Converter is used instead of 
uint_converter. uint_converter raises ValueError for negative integers, and 
_PyLong_UnsignedInt_Converter raises OverflowError.

Possible solutions:

1. Change tests. I don't like this, ValueError looks reasonable for these cases.

2. Continue to use uint_converter in Modules/zlibmodule.c. Then new converters 
become less useful.

3. Raise ValueError in new converters for negative integers. ValueError looks 
reasonable in many cases.

4. Raise ValueError in PyLong_AsUnsignedLong etc for negative integers.

Here is a patch which incorporates Larry's suggestions and implements option #3.

--
nosy: +mark.dickinson, skrah
Added file: http://bugs.python.org/file33483/clinic_unsigned_converters.patch

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue20260
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue20260] Argument Clinic: add unsigned integers converters

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

Oh, and of course 5th option: do nothing.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue20260
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue20260] Argument Clinic: add unsigned integers converters

[Stefan Krah]

Stefan Krah added the comment:

I think we cannot change PyLong_AsUnsignedLong() without a deprecation period,
if at all.  That leaves the option of changing the converters.

My preference is to raise either OverflowError or ValueError for *both*
out-of-range conditions.

People may me used to OverflowError by now -- the usage in 
PyLong_AsUnsignedLong()
dates back to very early revisions -- but there are equally good reasons to use
ValueError.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue20260
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue20260] Argument Clinic: add unsigned integers converters

[Larry Hastings]

Larry Hastings added the comment:

I'm glad you caught that!  First things first: the converted code should behave 
identically to the existing code, including raising the same exceptions.


If you examine the exception hierarchy:
http://docs.python.org/3.4/library/exceptions.html#exception-hierarchy

you'll see that OverflowError is a subclass of ArithmeticError.  In other 
words, it represents when you perform an arithmetic operation that overflows 
the result type.  Using it to also represent you specified a value that is out 
of range for this conversion seems wrong.

So I like #3 as well.

Could _PyLong_UnsignedInt_Converter catch the OverflowError raised by 
PyLong_AsUnsignedLong and reraise it as ValueError?

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue20260
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue20260] Argument Clinic: add unsigned integers converters

[Serhiy Storchaka]

New submission from Serhiy Storchaka:

Proposed patch adds support for non-bitwise unsigned integer arguments in 
Argument Clinic. I.e. now unsigned_int(bitwise=False) (or just unsigned_int) 
converts Python int in range from 0 to UINT_MAX to C unsigned int. Added 
support for unsigned_short, unsigned_int, unsigned_long, unsigned_PY_LONG_LONG, 
and size_t.

Also added global private functions _Py_UnsignedShort_Converter(), 
_Py_UnsignedInt_Converter(), _Py_UnsignedLong_Converter(), 
_Py_UnsignedLongLong_Converter(), and _Py_Size_t_Converter(), which are used by 
Argument Clinic and in C code still not converted to Argument Clinic.

--
components: Demos and Tools, Interpreter Core
files: clinic_unsigned_converters.patch
keywords: patch
messages: 208102
nosy: georg.brandl, larry, serhiy.storchaka
priority: normal
severity: normal
stage: patch review
status: open
title: Argument Clinic: add unsigned integers converters
type: enhancement
versions: Python 3.4
Added file: http://bugs.python.org/file33464/clinic_unsigned_converters.patch

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue20260
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue20260] Argument Clinic: add unsigned integers converters

[Larry Hastings]

Larry Hastings added the comment:

Very nice!  I had an idea to do this in the back of my head too.  But my plate 
is already full.

I haven't reviewed the patch yet--and I have a huge backlog of reviews already. 
 But I can give this higher priority if you need it for other patches.  When do 
you need this reviewed?

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue20260
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue20260] Argument Clinic: add unsigned integers converters

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

Currently these converters are used only in zlib (unsigned int) and select 
(unsigned short). But perhaps during conversion to Argument Clinic we will 
discover that they are more appropriate than bitwise converters in other 
places. So there is no hurry.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue20260
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com