Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Better _binary module use #4614

Closed
wants to merge 2 commits into from

Conversation

homm
Copy link
Member

@homm homm commented May 8, 2020

  • remove extra i8 calls where input is proved bytes[] or int. Most of i8 calls are not required in py3.
  • use offset for all binary input functions instead of slicing which should be slightly faster especially when slicing "until the end" is used (s[4:]).

linear = 0
else:
greyscale = 0
if self.mode in ["L", "LA", "P", "PA"]:
if greyscale:
if not linear:
self.lut = [i8(c) for c in palette[:256]]
self.lut = list(palette[:256])
Copy link
Member Author

Choose a reason for hiding this comment

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

In [1]: [c for c in b'test']
Out[1]: [116, 101, 115, 116]

In [2]: list(b'test')
Out[2]: [116, 101, 115, 116]

@homm
Copy link
Member Author

homm commented May 8, 2020

I don't have enough permissions to restart build on Travis (

@hugovk
Copy link
Member

hugovk commented May 8, 2020

I don't have enough permissions to restart build on Travis (

Maybe you do: try logging out and back in to Travis, that brought back the restart buttons for me recently.

@homm
Copy link
Member Author

homm commented May 8, 2020

Thanks, it works)

@@ -70,7 +69,7 @@ def read_32(fobj, start_length, size):
byte = fobj.read(1)
if not byte:
break
byte = i8(byte)
byte = byte[0]
Copy link
Member

Choose a reason for hiding this comment

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

Is it just me, or is byte = byte[0] not as clear as byte = i8(byte) about what is happening here?

Copy link
Member Author

Choose a reason for hiding this comment

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

You are right that i8() looks more clear at first glance. Mostly because indexing had changed from Py2 to Py3 for bytes. However, this saves one function call and type checking on each value. Also, IDEs handle types correctly and show variables as int.

Copy link
Member Author

Choose a reason for hiding this comment

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

As an alternative, it is possible to change i8 and use it everywhere for consistency:

def i8(c, o=0):
    return c[o]

@radarhere
Copy link
Member

radarhere commented Dec 30, 2020

I've created a rebased version of this as #5156

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants