[issue1023290] Conversion of longs to bytes and vice-versa.

[Mark Dickinson]

Mark Dickinson <dickinsm at gmail.com> added the comment:

The patch looks great!  Some comments:

- I think the type check for length_obj in long_tobytes should be more 
lenient:  I'd suggest a PyIndex_Check and PyNumber_AsSsize_t conversion 
instead of the PyLong_Check.  Or just use 'n' instead of 'O' in the 
PyArg_Parse* format;  this uses PyNumber_Index + PyLong_AsSsize_t, which 
amounts to the same thing (or at least I *think* it does).

- I like the pickle changes, but I think they should be committed 
separately.  (Unless they're somehow required for the rest of the patch 
to function correctly?)

- Stylistic nit:  There's some inconsistency in the NULL checks in the 
patch: "if (args != NULL)" versus "if (is_signed_obj)".  PEP 7 doesn't 
say anything about this, but the prevailing style in this file is for an 
explicit '== NULL' or '!= NULL'.

- I'm getting one failing test:

test.support.TestFailed: Traceback (most recent call last):
  File "Lib/test/test_long.py", line 1285, in test_frombytes
    self.assertRaises(TypeError, int.frombytes, "", 'big')
AssertionError: TypeError not raised by frombytes

This obviously has to do with issue 6687; as mentioned in that issue, 
I'm not sure that this should be an error.  How about just removing this 
test for now, pending a decision on that issue?

- Nice docs (and docstrings)!

On the subject of backporting to 2.7, I haven't seen any objections, so 
I'd say we should go for it.  (One argument for not backporting new 
features is to provide incentive for people to upgrade, but I can't 
realistically see this addition as a significant 'carrot'.)  I'm happy 
to do the backport, and equally happy to leave it to Alexandre if he's 
interested.

Leaving the bikeshedding until last:

Method names: I'm +0 on adding the extra underscore.  Python's already a 
bit inconsistent (e.g., float.fromhex and float.hex).  Also, at the time 
the float.fromhex and float.hex names were being discussed, 'hex' seemed 
to be preferred over 'tohex';  I wonder whether we should have int.bytes 
instead of int.to_bytes?

----------

_______________________________________
Python tracker <report at bugs.python.org>
<http://bugs.python.org/issue1023290>
_______________________________________