search

azaghal / pydenticon

Pydenticon is a small utility library that can be used for deterministically generating identicons based on the hash of provided data.
BSD 3-Clause "New" or "Revised" License
66 stars 10 forks source link

Pydenticon raises errors in Python3 #1

Closed baco closed 10 years ago

baco commented 10 years ago

I have written the following patch to add support to Python3:

diff --git a/docs/usage.rst b/docs/usage.rst
index 16b528a..59f943d 100644
--- a/docs/usage.rst
+++ b/docs/usage.rst
@@ -100,7 +100,7 @@ to achieve::
                                        output_format="png")

   # Identicon can be easily saved to a file.
-  f = open("sample.png", "w")
+  f = open("sample.png", "wb")
   f.write(identicon_png)
   f.close()

@@ -147,6 +147,6 @@ output them in PNG format to local directory::
                                    output_format="png")

     filename = user + ".png"
-    with open(filename, "w") as f:
+    with open(filename, "wb") as f:
         f.write(identicon)

diff --git a/pydenticon/__init__.py b/pydenticon/__init__.py
index 027d4a2..19506a4 100644
--- a/pydenticon/__init__.py
+++ b/pydenticon/__init__.py
@@ -64,8 +64,8 @@ class Generator(object):

         # Check if the digest produces sufficient entropy for identicon
         # generation.
-        entropy_provided = len(digest("test").hexdigest()) / 2 * 8
-        entropy_required = (columns / 2 + columns % 2) * rows + 8
+        entropy_provided = len(digest(b"test").hexdigest()) // 2 * 8
+        entropy_required = (columns // 2 + columns % 2) * rows + 8

         if entropy_provided < entropy_required:
             raise ValueError("Passed digest '%s' is not capable of providing %d bits of entropy" % (str(digest), entropy_required))
@@ -97,7 +97,7 @@ class Generator(object):
           True if the bit is 1. False if the bit is 0.
         """

-        if hash_bytes[n / 8] >> int(8 - ((n % 8) + 1)) & 1 == 1:
+        if hash_bytes[n // 8] >> int(8 - ((n % 8) + 1)) & 1 == 1:
             return True

         return False
@@ -119,7 +119,7 @@ class Generator(object):

         # Since the identicon needs to be symmetric, we'll need to work on half
         # the columns (rounded-up), and reflect where necessary.
-        half_columns = self.columns / 2 + self.columns % 2
+        half_columns = self.columns // 2 + self.columns % 2
         cells = self.rows * half_columns

         # Initialise the matrix (list of rows) that will be returned.
@@ -134,7 +134,7 @@ class Generator(object):
             if self._get_bit(cell, hash_bytes[1:]):

                 # Determine the cell coordinates in matrix.
-                column = cell / self.columns
+                column = cell // self.columns
                 row = cell % self.rows

                 # Mark the cell and its reflection. Central column may get
@@ -166,14 +166,14 @@ class Generator(object):

         # If data seems to provide identical amount of entropy as digest, it
         # could be a hex digest already.
-        if len(data) / 2 == self.digest_entropy / 8:
+        if len(data) // 2 == self.digest_entropy // 8:
             try:
                 data.decode("hex")
                 digest = data
             except TypeError:
                 digest = self.digest(data).hexdigest()
         else:
-            digest = self.digest(data).hexdigest()
+            digest = self.digest(data.encode('utf-8')).hexdigest()

         return [int(digest[i * 2:i * 2 + 2], 16) for i in range(16)]

@@ -217,8 +217,8 @@ class Generator(object):
         draw = ImageDraw.Draw(image)

         # Calculate the block widht and height.
-        block_width = width / self.columns
-        block_height = height / self.rows
+        block_width = width // self.columns
+        block_height = height // self.rows

         # Go through all the elements of a matrix, and draw the rectangles.
         for row, row_columns in enumerate(matrix):

The patch also corrects documentation.

azaghal commented 10 years ago

Thanks for the patch, I was thinking of messing with Python 3 support at some point, but wanted to postpone it a bit :)

Can you tell me if the above changes would work in Python 2.7 as well or not? If not, any suggestions on how to make the code compatible with both 2.7 and 3?

baco commented 10 years ago

Actually those changes work perfectly with Python 2.7, I've tried to change the least possible in order to keep that behavior.

I also noticed that the patch I sent to you before didn't work well in all the cases with Python 3 (it didn't work when the input was a hex hash), but only in Python 3; Python 2.7 behavior was not compromised.

Now I attach a new patch that includes the previous patch in it and also corrects the issue I've just mentioned.

baco commented 10 years ago

Ok, the e-mail didn't attach the patch file. Here it goes:

diff --git a/docs/usage.rst b/docs/usage.rst
index 16b528a..c8df87e 100644
--- a/docs/usage.rst
+++ b/docs/usage.rst
@@ -83,7 +83,7 @@ Finally, the resulting identicons can be in different formats::
                                      output_format="png")
   # Create identicon in ASCII format.
   identicon_ascii = generator.generate("john.doe@example.com", 200, 200,
-                                       output_format="png")
+                                       output_format="ascii")

 Using the generated identicons
 ------------------------------
@@ -97,10 +97,10 @@ to achieve::
   identicon_png = generator.generate("john.doe@example.com", 200, 200,
                                      output_format="png")
   identicon_ascii = generator.generate("john.doe@example.com", 200, 200,
-                                       output_format="png")
+                                       output_format="ascii")

   # Identicon can be easily saved to a file.
-  f = open("sample.png", "w")
+  f = open("sample.png", "wb")
   f.write(identicon_png)
   f.close()

@@ -147,6 +147,6 @@ output them in PNG format to local directory::
                                    output_format="png")

     filename = user + ".png"
-    with open(filename, "w") as f:
+    with open(filename, "wb") as f:
         f.write(identicon)

diff --git a/pydenticon/__init__.py b/pydenticon/__init__.py
index 027d4a2..41850a3 100644
--- a/pydenticon/__init__.py
+++ b/pydenticon/__init__.py
@@ -7,6 +7,8 @@ from io import BytesIO
 # Pillow for Image processing.
 from PIL import Image, ImageDraw

+import binascii
+

 class Generator(object):
     """
@@ -64,8 +66,8 @@ class Generator(object):

         # Check if the digest produces sufficient entropy for identicon
         # generation.
-        entropy_provided = len(digest("test").hexdigest()) / 2 * 8
-        entropy_required = (columns / 2 + columns % 2) * rows + 8
+        entropy_provided = len(digest(b"test").hexdigest()) // 2 * 8
+        entropy_required = (columns // 2 + columns % 2) * rows + 8

         if entropy_provided < entropy_required:
             raise ValueError("Passed digest '%s' is not capable of providing %d bits of entropy" % (str(digest), entropy_required))
@@ -97,7 +99,7 @@ class Generator(object):
           True if the bit is 1. False if the bit is 0.
         """

-        if hash_bytes[n / 8] >> int(8 - ((n % 8) + 1)) & 1 == 1:
+        if hash_bytes[n // 8] >> int(8 - ((n % 8) + 1)) & 1 == 1:
             return True

         return False
@@ -119,7 +121,7 @@ class Generator(object):

         # Since the identicon needs to be symmetric, we'll need to work on half
         # the columns (rounded-up), and reflect where necessary.
-        half_columns = self.columns / 2 + self.columns % 2
+        half_columns = self.columns // 2 + self.columns % 2
         cells = self.rows * half_columns

         # Initialise the matrix (list of rows) that will be returned.
@@ -134,7 +136,7 @@ class Generator(object):
             if self._get_bit(cell, hash_bytes[1:]):

                 # Determine the cell coordinates in matrix.
-                column = cell / self.columns
+                column = cell // self.columns
                 row = cell % self.rows

                 # Mark the cell and its reflection. Central column may get
@@ -166,14 +168,14 @@ class Generator(object):

         # If data seems to provide identical amount of entropy as digest, it
         # could be a hex digest already.
-        if len(data) / 2 == self.digest_entropy / 8:
+        if len(data) // 2 == self.digest_entropy // 8:
             try:
-                data.decode("hex")
-                digest = data
-            except TypeError:
-                digest = self.digest(data).hexdigest()
+                binascii.hexlify(data.encode('utf-8'))
+                digest = data.encode('utf-8')
+            except (TypeError, binascii.Error):
+                digest = self.digest(data.encode('utf-8')).hexdigest()
         else:
-            digest = self.digest(data).hexdigest()
+            digest = self.digest(data.encode('utf-8')).hexdigest()

         return [int(digest[i * 2:i * 2 + 2], 16) for i in range(16)]

@@ -217,8 +219,8 @@ class Generator(object):
         draw = ImageDraw.Draw(image)

         # Calculate the block widht and height.
-        block_width = width / self.columns
-        block_height = height / self.rows
+        block_width = width // self.columns
+        block_height = height // self.rows

         # Go through all the elements of a matrix, and draw the rectangles.
         for row, row_columns in enumerate(matrix):
azaghal commented 10 years ago

Ok, I was planning on applying your patches locally and running the tests to see if there's any updates needed, but got side-tracked by other things :)

To be on a bit of a demanding side - have you ran the test suite for Pydenticon perhaps? Were any tests failing after the patch if you did?

azaghal commented 10 years ago

Hm... This line is not correct at least:

binascii.hexlify(data.encode('utf-8'))

It should instead be:

bytes.fromhex(data)

Of course, different exception needs to be caught there as well then. The purpose of that line in particular is to actually fail if the passed data has length that reminds of digested data, but is not actually a valid digest.

azaghal commented 10 years ago

Ah... Got it, it should be unhexlify there :)

azaghal commented 10 years ago

@baco I have finally gotten around to merge your patch, but before I do that I wanted to check with you if it's ok if I specify you as author in the form of Dionisio E Alonso (I haven't got your mail, so thought on using your nickname). Please reply quickly, I'd like to make a push of this commit :)

baco commented 10 years ago

I think it's nice that you specify me as the author. The e-mail is dalonso at grulic.org.ar, you could use Dionisio E Alonso (Baco) <dalonso at grulic.org.ar> for instance.

On 13 July 2014 18:14, Branko Majic notifications@github.com wrote:

@baco https://github.com/baco I have finally gotten around to merge your patch, but before I do that I wanted to check with you if it's ok if I specify you as author in the form of Dionisio E Alonso (I haven't got your mail, so thought on using your nickname). Please reply quickly, I'd like to make a push of this commit :)

I am very glad I could help in with come coding.

best regards,

Dionisio.

baco commented 10 years ago

On 13 July 2014 19:37, Dionisio E Alonso dalonso@adiuc.org.ar wrote:

I think it's nice that you specify me as the author. The e-mail is dalonso at grulic.org.ar, you could use Dionisio E Alonso (Baco) <dalonso at grulic.org.ar> for instance.

On 13 July 2014 18:14, Branko Majic notifications@github.com wrote:

@baco I have finally gotten around to merge your patch, but before I do that I wanted to check with you if it's ok if I specify you as author in the form of Dionisio E Alonso (I haven't got your mail, so thought on using your nickname). Please reply quickly, I'd like to make a push of this commit :)

The github NickName @baco it's also good. :-)

Dionisio

azaghal commented 10 years ago

Ok, pushed the changes to github and my own repo server. I consider this closed for now - only need to release 0.2 perhaps in 1-2 weeks when I get some time :)