gen_integer() change #62
Conversation
|
@apense awesome! Could you squash your commits so I can merge it? |
|
Should be done. I'm really new to git, so if there's a better way to do this, let me know & I'll look it up. |
|
No worries and thank you for doing this. If I wasn't on the phone now I could provide more info on squashing and stuff like that. Your commit message now only mentions you adding yourself to the AUTHORS file. If you want to change it to mention the real fix I can wait before merging it. I will then release a new version tomorrow with it ;) Thank you for your help! |
|
Updated with a more sensible, general message. |
| @@ -342,10 +342,15 @@ def gen_integer(min_value=None, max_value=None): | |||
| if max_value is None: | |||
| max_value = _max_value | |||
|
|
|||
| if sys.version < '3': | |||
There was a problem hiding this comment.
I am finally at my computer now and was thinking, how using:
if sys.version_info.major < 3:I only now realized that sys.version returns a string and I feel that comparing a string is a bit weird...
sys.version
'2.7.6 (default, Sep 9 2014, 15:04:36) \n[GCC 4.2.1 Compatible Apple LLVM 6.0 (clang-600.0.39)]'There was a problem hiding this comment.
Sorry I've been writing an extension in C and confused the two haha.
# Python 2 and 3: option 1
from builtins import int # subclass of long on Py2
if isinstance(x, int): # matches both int and long on Py2
...
# Python 2 and 3: option 2
from past.builtins import long
if isinstance(x, (int, long)):
This is the recommendation of python-future.org. Do you have a preference?
I think your recommendation is cleaner, though. So I'm switching over to that.
On Thu, Dec 18, 2014 at 1:16 PM, Jonathan Edwards apense@gmail.com wrote:
Agreed. Sometimes people use
SYS_VERSION_HEX < 0x03000000
instead. I'll take a look around and see if there's a cleaner way of doing
it.
On Dec 18, 2014 9:26 AM, "Og Maciel" notifications@github.com wrote:In fauxfactory/init.py
#62 (diff)
:@@ -342,10 +342,15 @@ def gen_integer(min_value=None, max_value=None):
if max_value is None:
max_value = _max_value
- if sys.version < '3':
I am finally at my computer now and was thinking, how using:
if sys.version_info.major < 3:
I only now realized that sys.version returns a string and I feel that
comparing a string is a bit weird...sys.version'2.7.6 (default, Sep 9 2014, 15:04:36) \n[GCC 4.2.1 Compatible Apple LLVM 6.0 (clang-600.0.39)]'
—
Reply to this email directly or view it on GitHub
https://github.com/omaciel/fauxfactory/pull/62/files#r22043994.
|
Test as |
Added Jonathan Edwards to AUTHORS Updated gen_integer() to not throw error on long sys.maxsize Updated test to reflect changes in gen_integer() Updated isinstance() test to match all integer types Updated to reflect change in gen_integer() Fixed test for Python 3 to a more sensible alternative.
|
Squashed new change. |
|
ACK and merge from my phone ftw! :) Thank you! |
Using Win64 Python2.7, sys.maxsize returns a long and the
isinstance(maxsize, int)test fails. I updated this to a tuple to consider the differences with Python3 (which doesn't really uselong()) by adding an integer types tuple(int, long,)depending on the platform.