python-intelhex / intelhex

Python IntelHex library
BSD 3-Clause "New" or "Revised" License
198 stars 106 forks source link

tostring bug when using Python 3.9 #45

Closed corb5309 closed 3 years ago

corb5309 commented 3 years ago

https://github.com/python-intelhex/intelhex/blob/9769d3f6e1beefa3bd0c2b266d60197d61669881/intelhex/compat.py#L60

tostring has to become tobytes for this to work in Python 3.9, likely in versions before this as well.

ghost commented 3 years ago

Can confirm this issue on Python 3.9.

Traceback if necessary:

Traceback (most recent call last):
  File "/Users/ashley/Library/Python/3.9/bin/solo", line 5, in <module>
    from solo.cli import solo_cli
  File "/Users/ashley/Library/Python/3.9/lib/python/site-packages/solo/__init__.py", line 15, in <module>
    from . import client, commands, dfu, helpers, operations
  File "/Users/ashley/Library/Python/3.9/lib/python/site-packages/solo/client.py", line 27, in <module>
    from intelhex import IntelHex
  File "/Library/Frameworks/Python.framework/Versions/3.9/lib/python3.9/site-packages/intelhex/__init__.py", line 44, in <module>
    from intelhex.compat import (
  File "/Library/Frameworks/Python.framework/Versions/3.9/lib/python3.9/site-packages/intelhex/compat.py", line 60, in <module>
    array_tobytes = getattr(array.array, "tobytes", array.array.tostring)
AttributeError: type object 'array.array' has no attribute 'tostring'
fernandez85 commented 3 years ago

tobytes was introduced in ver 3.2, and since then tostring is deprecated alias for tobytes current implementation isn't ready for situation when it's completely removed, and that what happened in ver 3.9

MaximumSU commented 3 years ago

@fernandez85 i think, the fix is simple. array_tobytes = getattr(array.array, "tobytes", array.array.tostring) to: array_tobytes = getattr(array.array, "tostring", array.array.tobytes)

fernandez85 commented 3 years ago

I was thinking about this at first time, but then same problem will be with python ver 3.0.x, and 3.1.x. I know there shouldn't be many users of these versions, but best way is to make it right :) This should do the trick (in that exact place): array_tobytes = array.array.tobytes if sys.version_info[1] >= 2 else array.array.tostring

or use hasattr

MaximumSU commented 3 years ago

getattr(array.array, "tostring", array.array.tobytes)

array.array.tobytes if sys.version_info[1] >= 2 else array.array.tostring

IIt does't matter, the result is the same. But for me is interesting:

@fernandez85 I was thinking about this at first time, but then same problem will be with python ver 3.0.x, and 3.1.x.

Why? All python < 3.9 has "tostring", for python >=3.9 it will use the default as "tobytes".

omerk commented 3 years ago

Thank you @The-42 for merging the PR that fixes this.

Is there any change a new release could be cut and the PyPi package updated as well as most folks just pull intelhex as a dependency through pip?

Thanks in advance.

fernandez85 commented 3 years ago

getattr(array.array, "tostring", array.array.tobytes)

array.array.tobytes if sys.version_info[1] >= 2 else array.array.tostring

IIt does't matter, the result is the same. But for me is interesting:

It does matter, because it isn't the same. It wouldn't matter for >=3.2 only, but then we wouldn't need getattr anymore

@fernandez85 I was thinking about this at first time, but then same problem will be with python ver 3.0.x, and 3.1.x.

Why? All python < 3.9 has "tostring", for python >=3.9 it will use the default as "tobytes".

But not all has tobytes - it was introduced in ver 3.2 Your proposal simply causes the same problem as it was at first place,... but in opposite, only for those that still uses python 3.0-3.1

MaximumSU commented 3 years ago

@fernandez85

But not all has tobytes - it was introduced in ver 3.2

And what? It has tostring and it will be used, othervise tobyte will be used

If the named attribute does not exist, default is returned

getattr

getattr(object, name[, default]) Return the value of the named attribute of object. name must be a string. If the string is the name of one of the object’s attributes, the result is the value of that attribute. For example, getattr(x, 'foobar') is equivalent to x.foobar. If the named attribute does not exist, default is returned if provided, otherwise AttributeError is raised.

fernandez85 commented 3 years ago

Do you know why even this issue exist? Because in this code: getattr(array.array, "tobytes", array.array.tostring) tostring is not available in 3.9, even if tobytes is present, because default argument is processed anyway and it simply doesn't exist, that is why exception was thrown the same will be (but opposite) with: array_tobytes = getattr(array.array, "tostring", array.array.tobytes) Is this really hard to understand?

In my solution only one attribute is processed, because of if statement

MaximumSU commented 3 years ago

@fernandez85

because defualt argument is process anyway

Aha... Thanks, now it is cleare.

The-42 commented 3 years ago

Thank you @The-42 for merging the PR that fixes this.

Is there any change a new release could be cut and the PyPi package updated as well as most folks just pull intelhex as a dependency through pip?

Thanks in advance.

I'll see to packing a release soon, still need to check on a couple issues though. Uploading to PyPi is currently out of my hands as @bialix is the Pypi maintainer for this package.

The-42 commented 3 years ago

Update: We've figured out PyPi, that isn't a showstopper anymore :)

The-42 commented 3 years ago

Okay folks, you should see a new release on pypi. pip seems to have found it:

$ pip search intelhex
intelhex (2.3.0)  - Python library for Intel HEX files manipulations
...

readthedocs.io has not been updated yet, but that wasn't a priority :)

omerk commented 3 years ago

Thank you @The-42 for the expedited release :-)