When data_or_wLength is an array, keep the same object#527
When data_or_wLength is an array, keep the same object#527kauwua wants to merge 1 commit intopyusb:masterfrom
Conversation
1a3332e to
7e00c5c
Compare
| except TypeError: | ||
| buff = util.create_buffer(data_or_wLength) |
There was a problem hiding this comment.
Hey, thanks for the report.
I think that depending on TypeError being raised is fragile, and continuing to do so would be a mistake.
How about:
if isinstance(data_or_wLength, int):
buff = util.create_buffer(data_or_wLength)
else:
buff = _interop.as_array(data_or_wLength)There was a problem hiding this comment.
I agree that waiting for a TypeError to happen is not clean, however it might be faster than checking the type of data_or_wLength at each transfer. A good thing with the fix is that it puts as_array first in the try block, which means if someone gives an existing array there is little overhead, while keeping an overhead for the case where an array is created each time anyways (so there's already an overhead).
In case you prefer using isinstance, checking data_or_wLength against the abtract numbers.Integral type might be better(https://peps.python.org/pep-3141/), I checked that isinstance(x, int) does not work with Numpy (isinstance(numpy.uint64(3), int) doesn't work for instance) but isinstance(x,numbers.Integral) works and is more generic:
import numbers
if isinstance(data_or_wLength, numbers.Integral):
buff = util.create_buffer(data_or_wLength)
else:
buff = _interop.as_array(data_or_wLength)There was a problem hiding this comment.
Optimizing for the case where a byte array is passed in and not breaking the support for numpy integers are both valid points, thanks! But considering them, I think maybe #530 is a more comprehensive fix/improvement than either this or #529.
Can you take a look at #530 and see if I missed anything, or if you have any other suggestions/concerns?
While discussing pyusb#527, @kauwua made the valid point that we could optimize for the case where the user has passed in a byte `array`. In that spirit, prioritize the no-op conversion case, but also accept the parameter to be a length for a new array in preparation for replacing the conditional in `Device.ctrl_transfer`. Related: pyusb#528
While discussing pyusb#527, @kauwua made the valid point that we could optimize for the case where the user has passed in a byte `array`. In that spirit, prioritize the no-op conversion case, but also accept the parameter to be a length for a new array in preparation for replacing the conditional in `Device.ctrl_transfer`. Related: pyusb#528
While discussing pyusb#527, @kauwua made the valid point that we could optimize for the case where the user has passed in a byte `array`. In that spirit, prioritize the no-op conversion case, but also accept the parameter to be a length for a new array in preparation for replacing the conditional in `Device.ctrl_transfer`. Related: pyusb#528
While discussing #527, @kauwua made the valid point that we could optimize for the case where the user has passed in a byte `array`. In that spirit, prioritize the no-op conversion case, but also accept the parameter to be a length for a new array in preparation for replacing the conditional in `Device.ctrl_transfer`. Related: #528
In
v1.2.1, when data_or_wLength was an array, aTypeErrorexception was triggered increate_bufferwhich means_interop.as_arraywas called and the same array object was used (modified in-place as expected).In
v1.3.0, commit 3ea79b0 introduced a change increate_bufferthat does not trigger theTypeErrorexception and instead creates a new array from the one passed indata_or_wLength.This means a new array is created and receives data for IN transfers, not the array passed as argument.