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

Update to Python 3.9 #1690

Merged
merged 2 commits into from Dec 7, 2020
Merged

Update to Python 3.9 #1690

merged 2 commits into from Dec 7, 2020

Conversation

mikacousin
Copy link
Contributor

Fix AttributeError: 'array.array' object has no attribute 'fromstring'

Fix AttributeError: 'array.array' object has no attribute 'fromstring'
@peternewman peternewman changed the base branch from 0.10.8 to 0.10 December 2, 2020 14:11
@peternewman peternewman self-assigned this Dec 2, 2020
@peternewman peternewman added this to the 0.10.9 milestone Dec 2, 2020
Copy link
Member

@peternewman peternewman left a comment

Choose a reason for hiding this comment

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

Thanks @mikacousin . Good spot.

I've updated your PR to target the 0.10 branch. We work in the master/0.x branches and just generate the 0.x.y branches at release for historical reasons.

Would you be interested in adding a test of one or both of these functions to ClientWrapperTest.py in the same way as @brucelowekamp did in there and RDMTest.py as it seems this issue slipped through the net due to having no equivalent test for these functions.

@mikacousin
Copy link
Contributor Author

Would you be interested in adding a test of one or both of these functions to ClientWrapperTest.py in the same way as @brucelowekamp did in there and RDMTest.py as it seems this issue slipped through the net due to having no equivalent test for these functions.

I'm sorry but I don't master ola and python enough to write those tests. Shame on me, but I still have a lot to learn :-)

Copy link
Contributor

@brucelowekamp brucelowekamp left a comment

Choose a reason for hiding this comment

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

Good catch. I hate these if version things, though. Would this work with just:
data = array.array('B', request.data)
?

@mikacousin
Copy link
Contributor Author

@brucelowekamp : Just tested with python 3.9, it works and it's more elegant.

@brucelowekamp
Copy link
Contributor

Would you be interested in adding a test of one or both of these functions to ClientWrapperTest.py in the same way as @brucelowekamp did in there and RDMTest.py as it seems this issue slipped through the net due to having no equivalent test for these functions.

I'm sorry but I don't master ola and python enough to write those tests. Shame on me, but I still have a lot to learn :-)

@peternewman It would definitely be a bit challenging to write the tests, though I think the socket mock could do it. I don't have time for now. IMHO it would be better to merge this with a TODO to write tests for it. I believe the change should work on python2 and python3.

@peternewman
Copy link
Member

@peternewman It would definitely be a bit challenging to write the tests, though I think the socket mock could do it. I don't have time for now. IMHO it would be better to merge this with a TODO to write tests for it. I believe the change should work on python2 and python3.

I think you've already done it with the RDM ones that send and receive data @brucelowekamp . I was actually going to try and knock one together and see if @mikacousin or you could test it instead but I keep getting sidetracked today.

@peternewman
Copy link
Member

Hmm, I thought I'd covered this test in c402fcb but it's still passing on Python 3.7 which it looks like it shouldn't from the docs, so I guess I've not tested quite the right bit...

@peternewman
Copy link
Member

Hmm, I thought I'd covered this test in c402fcb but it's still passing on Python 3.7 which it looks like it shouldn't from the docs, so I guess I've not tested quite the right bit...

Okay I'm really confused now, because a local run with some print statements proves it is running the array code in my test. @mikacousin could you give that commit/my branch a test on your machine please?

@mikacousin
Copy link
Contributor Author

mikacousin commented Dec 6, 2020

No problem, compiling now your clang-latest branch.
make check failed with fromstring, that seems good:

==================================
   OLA 0.10.8: ./test-suite.log
==================================

# TOTAL: 94
# PASS:  93
# SKIP:  0
# XFAIL: 0
# FAIL:  1
# XPASS: 0
# ERROR: 0

.. contents:: :depth: 2

FAIL: python/ola/ClientWrapperTest.sh
=====================================

....EE.
======================================================================
ERROR: testFetchDmx (__main__.ClientWrapperTest)
uses client to send a FetchDMX with mocked olad.
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/mika/Projects/Ola/ola-0.10-clang-latest/./python/ola/ClientWrapperTest.py", line 367, in testFetchDmx
    wrapper.Run()
  File "/home/mika/Projects/Ola/ola-0.10-clang-latest/python/ola/ClientWrapper.py", line 312, in Run
    self._ss.Run()
  File "/home/mika/Projects/Ola/ola-0.10-clang-latest/python/ola/ClientWrapper.py", line 232, in Run
    self._CheckDescriptors(i, self._read_descriptors)
  File "/home/mika/Projects/Ola/ola-0.10-clang-latest/python/ola/ClientWrapper.py", line 267, in _CheckDescriptors
    runnable()
  File "/home/mika/Projects/Ola/ola-0.10-clang-latest/python/ola/OlaClient.py", line 803, in SocketReady
    self._channel.SocketReady()
  File "/home/mika/Projects/Ola/ola-0.10-clang-latest/python/ola/rpc/StreamRpcChannel.py", line 88, in SocketReady
    self._ProcessIncomingData()
  File "/home/mika/Projects/Ola/ola-0.10-clang-latest/python/ola/rpc/StreamRpcChannel.py", line 269, in _ProcessIncomingData
    self._HandleNewMessage(data)
  File "/home/mika/Projects/Ola/ola-0.10-clang-latest/python/ola/rpc/StreamRpcChannel.py", line 283, in _HandleNewMessage
    self.MESSAGE_HANDLERS[message.type](self, message)
  File "/home/mika/Projects/Ola/ola-0.10-clang-latest/python/ola/rpc/StreamRpcChannel.py", line 327, in _HandleResponse
    self._InvokeCallback(response)
  File "/home/mika/Projects/Ola/ola-0.10-clang-latest/python/ola/rpc/StreamRpcChannel.py", line 370, in _InvokeCallback
    response.callback(response.controller, response.reply)
  File "/home/mika/Projects/Ola/ola-0.10-clang-latest/python/ola/OlaClient.py", line 932, in <lambda>
    lambda x, y: self._GetDmxComplete(callback, x, y))
  File "/home/mika/Projects/Ola/ola-0.10-clang-latest/python/ola/OlaClient.py", line 1481, in _GetDmxComplete
    data.fromstring(response.data)
AttributeError: 'array.array' object has no attribute 'fromstring'

======================================================================
ERROR: testRegisterUniverse (__main__.ClientWrapperTest)
uses client to send a DMX register universe with mocked olad.
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/mika/Projects/Ola/ola-0.10-clang-latest/./python/ola/ClientWrapperTest.py", line 300, in testRegisterUniverse
    wrapper.Run()
  File "/home/mika/Projects/Ola/ola-0.10-clang-latest/python/ola/ClientWrapper.py", line 312, in Run
    self._ss.Run()
  File "/home/mika/Projects/Ola/ola-0.10-clang-latest/python/ola/ClientWrapper.py", line 232, in Run
    self._CheckDescriptors(i, self._read_descriptors)
  File "/home/mika/Projects/Ola/ola-0.10-clang-latest/python/ola/ClientWrapper.py", line 267, in _CheckDescriptors
    runnable()
  File "/home/mika/Projects/Ola/ola-0.10-clang-latest/python/ola/OlaClient.py", line 803, in SocketReady
    self._channel.SocketReady()
  File "/home/mika/Projects/Ola/ola-0.10-clang-latest/python/ola/rpc/StreamRpcChannel.py", line 88, in SocketReady
    self._ProcessIncomingData()
  File "/home/mika/Projects/Ola/ola-0.10-clang-latest/python/ola/rpc/StreamRpcChannel.py", line 269, in _ProcessIncomingData
    self._HandleNewMessage(data)
  File "/home/mika/Projects/Ola/ola-0.10-clang-latest/python/ola/rpc/StreamRpcChannel.py", line 283, in _HandleNewMessage
    self.MESSAGE_HANDLERS[message.type](self, message)
  File "/home/mika/Projects/Ola/ola-0.10-clang-latest/python/ola/rpc/StreamRpcChannel.py", line 314, in _HandleRequest
    self._service.CallMethod(method, request.controller, request_pb,
  File "/usr/lib/python3.9/site-packages/google/protobuf/service_reflection.py", line 161, in _WrapCallMethod
    return self._CallMethod(srvc, method_descriptor,
  File "/usr/lib/python3.9/site-packages/google/protobuf/service_reflection.py", line 187, in _CallMethod
    return method(rpc_controller, request, callback)
  File "/home/mika/Projects/Ola/ola-0.10-clang-latest/python/ola/OlaClient.py", line 1176, in UpdateDmxData
    data.fromstring(request.data)
AttributeError: 'array.array' object has no attribute 'fromstring'

----------------------------------------------------------------------
Ran 7 tests in 0.018s

FAILED (errors=2)
FAIL python/ola/ClientWrapperTest.sh (exit status: 1)

Do you need more tests ?

PS: If I remove fromstring in OlaClient.py, make check pass without any error

@peternewman
Copy link
Member

That's perfect thanks @mikacousin . Sorry I was thrown by it working in 3.7 but not in 3.9, but having read more carefully I see it was deprecating in 3.2 and dropped in 3.9.

Copy link
Member

@peternewman peternewman left a comment

Choose a reason for hiding this comment

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

Thanks again @mikacousin . Works in reality, passes my new tests, what's not to like!

@peternewman
Copy link
Member

Force merging as we've got some outstanding Travis issues which will be fixed in another PR by me, no point holding this one up.

@peternewman peternewman merged commit 1853b4c into OpenLightingProject:0.10 Dec 7, 2020
@peternewman
Copy link
Member

@brucelowekamp I assume you don't have a nicer way you'd prefer to do this bit which we should ask @mikacousin nicely to test on a machine with 3.9?:

ola/python/ola/OlaClient.py

Lines 949 to 952 in 7090ab2

if sys.version >= '3.2':
request.data = data.tobytes()
else:
request.data = data.tostring()

@brucelowekamp
Copy link
Contributor

@brucelowekamp I assume you don't have a nicer way you'd prefer to do this bit which we should ask @mikacousin nicely to test on a machine with 3.9?:

ola/python/ola/OlaClient.py

Lines 949 to 952 in 7090ab2

if sys.version >= '3.2':
request.data = data.tobytes()
else:
request.data = data.tostring()

not that I know of. problems with renaming methods across releases...

@peternewman
Copy link
Member

not that I know of. problems with renaming methods across releases...

Okay thanks. Yeah this Python 3 thing has certainly been painful in terms of migration. Although I guess things like libmicrohttpd didn't do much better either.

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

Successfully merging this pull request may close these issues.

None yet

3 participants