Skip to content
This repository was archived by the owner on Apr 20, 2025. It is now read-only.

Create PY2 constant to simplify compatibility decisions #82

Merged
merged 1 commit into from
Jan 15, 2017
Merged

Create PY2 constant to simplify compatibility decisions #82

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 17 additions & 10 deletions rsa/_compat.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@
MAX_INT32 = (1 << 31) - 1
MAX_INT16 = (1 << 15) - 1

PY2 = sys.version_info[0] == 2

# Determine the word size of the processor.
if MAX_INT == MAX_INT64:
# 64-bit processor.
Expand All @@ -37,19 +39,24 @@
# Else we just assume 64-bit processor keeping up with modern times.
MACHINE_WORD_SIZE = 64

# Range generator.
try:
# < Python3
if PY2:
integer_types = (int, long)
range = xrange
except NameError:
# Python3
else:
integer_types = (int, )
range = range

# ``long`` is no more. Do type detection using this instead.
try:
integer_types = (int, long)
except NameError:
integer_types = (int,)

def write_to_stdout(data):
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe add a docstring like this?

"""Writes bytes to stdout

:type data: bytes
"""

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure!

"""Writes bytes to stdout

:type data: bytes
"""
if PY2:
sys.stdout.write(data)
else:
# On Py3 we must use the buffer interface to write bytes.
sys.stdout.buffer.write(data)


def is_bytes(obj):
Expand Down
12 changes: 2 additions & 10 deletions rsa/cli.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,11 +83,7 @@ def keygen():
outfile.write(data)
else:
print('Writing private key to stdout', file=sys.stderr)
if sys.version_info[0] >= 3:
# on Py3 we must use the buffer interface to write bytes.
sys.stdout.buffer.write(data)
else:
sys.stdout.write(data)
rsa._compat.write_to_stdout(data)


class CryptoOperation(object):
Expand Down Expand Up @@ -193,11 +189,7 @@ def write_outfile(self, outdata, outname):
outfile.write(outdata)
else:
print('Writing output to stdout', file=sys.stderr)
if sys.version_info[0] >= 3:
# on Py3 we must use the buffer interface to write bytes.
sys.stdout.buffer.write(outdata)
else:
sys.stdout.write(outdata)
rsa._compat.write_to_stdout(outdata)


class EncryptOperation(CryptoOperation):
Expand Down
26 changes: 12 additions & 14 deletions tests/test_cli.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,25 +15,23 @@
import rsa
import rsa.cli
import rsa.util
from rsa._compat import PY2

if sys.version_info[0] < 3:
def make_buffer():

def make_buffer():
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmmm now that I see how it turns out, I have some doubts about doing the if PY3 inside the function. The way it is now, if PY3 is evaluated for each and every function call. By doing something like this:

if PY3:
    def write_to_stdout(data): ...
    def make_buffer(): ...
else:
    def write_to_stdout(data): ...
    def make_buffer(): ...

we only need to evaluate if PY3 once. This is slightly harder to read, as the PY3 and PY2 versions of the functions are potentially further apart, it does make the runtime execution a bit faster. What do you think @adamantike ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I try to prioritize readability and DRYness instead of (over)optimization. Even if these functions don't share logic, I prefer to have a unique point of "failure", one docstring to maintain and shared common blocks inside the function if that's possible.

In my opinion, those advantages will make devs to save a lot more time when trying to contribute to the project or follow a stack trace than one more comparison per call will ever provide. Does that make any sense?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It makes sense, and I agree that readability is very important. However, I'm also curious as to the runtime performance impact. Do you have time to do some timing tests with some large key generation and file encryption?

if PY2:
return BytesIO()
buf = StringIO()
buf.buffer = BytesIO()
return buf


def get_bytes_out(out):
# Python 2.x writes 'str' to stdout:
def get_bytes_out(out):
if PY2:
# Python 2.x writes 'str' to stdout
return out.getvalue()
else:
def make_buffer():
buf = StringIO()
buf.buffer = BytesIO()
return buf


def get_bytes_out(out):
# Python 3.x writes 'bytes' to stdout.buffer:
return out.buffer.getvalue()
# Python 3.x writes 'bytes' to stdout.buffer
return out.buffer.getvalue()


@contextmanager
Expand Down
1 change: 0 additions & 1 deletion tests/test_compat.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@

import unittest
import struct
import sys

from rsa._compat import byte, is_bytes, range

Expand Down