Skip to content

Fix _struct_ and _array_ format strings in Python 3#5

Closed
vadmium wants to merge 2 commits intowaveform-computing:masterfrom
vadmium:master
Closed

Fix _struct_ and _array_ format strings in Python 3#5
vadmium wants to merge 2 commits intowaveform-computing:masterfrom
vadmium:master

Conversation

@vadmium
Copy link

@vadmium vadmium commented Apr 1, 2014

Hi, I saw this project mentioned in an OleFileIO bug, and thought I’d try it out. This fixes some issues I had, though I admit I haven’t tested it under Python 2 etc. In Python 3 (3.4 at least), array() does not accept byte string typecodes. Although struct() does accept byte format strings, it is not well documented; see https://bugs.python.org/issue16349.

Usually when supporting both Python 2 and 3, I use code like struct(str(format)), but in this case I had to restore the proper Python 2 str() type for that to work. These changes are just a suggestion; I don’t mind if you fix it another way, though I do think it’s misleading to change the meaning of “str” in Python 2 :P (And beware that I did not restore it in one or two files.)

Martin Panter added 2 commits April 1, 2014 08:10
Use “basestring” for type checking instead.
In Python 2, Unicode is not accepted for format strings, but it is preferred
Python 3.

Previously, the code was calling bytes() incorrectly, resulting in:

    TypeError: string argument without an encoding

Also, Python 3 does not accept a byte string for “array”, resulting in:

    TypeError: must be a unicode character, not bytes
@waveform80
Copy link
Member

Good catch! A proper test suite is something I'm trying to find the time to write so I haven't really concentrated on Python 3 compatibility yet, although it's something I try and bake in from the start on most projects (hence my dirty hacks with str which allow me to use idioms like "if isinstance(foo, (str, bytes))" just like one would in py3).

Anyway, thanks for the pull request - I'll try and find some time this afternoon to look at it properly!

@waveform80
Copy link
Member

Sorry for the long delay in this - too many projects at the mo! For now, I've decided to go with a slightly different fix which keeps the (admittedly dirty) hack on str. My reasoning goes like this: the goal is to have a single code-base that supports both Python 2 and Python 3; given that Python 3 is the future of the language I'd rather use compatibility hacks that make Python 2 as close as possible to idiomatic Python 3 rather than use hacks which make Python 3 closer to Python 2. Then, in the dim and distant future, when Python 2 is eventually retired the hacks can be removed from the modules and what will remain will look as if it were written for Python 3 in the first place (which in a sense it is). Still, many thanks for the pull request!

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants