BFs for visa_communicator and tests #308
Conversation
- read_raw() now uses read_raw() of pyvisa and not read() - Not implemented functions now raise NotImplementedError - Removed testing if pyvisa is present
| @@ -29,9 +29,6 @@ class VisaCommunicator(io.IOBase, AbstractCommunicator): | |||
| def __init__(self, conn): | |||
| super(VisaCommunicator, self).__init__(self) | |||
|
|
|||
| if pyvisa is None: | |||
| raise ImportError("PyVISA required for accessing VISA instruments.") | |||
|
|
|||
There was a problem hiding this comment.
Not sure this can ever be triggered, wouldn't it throw an error during the import pyvisa above if pyvisa is not installed?
There was a problem hiding this comment.
Indeed it would. Also, pyvisa is named as a dependency of IK, so I'm not sure in what scenario your users would end up with pyvisa not available?
In any case, I think the normal pattern for "optional" packages is doing something like this above:
try:
import pyvisa
except ImportError:
pyvisa = None
, then this would work as you expect (I think).
There was a problem hiding this comment.
In addition, just noticed that the same check is also in instrument.py before opening the visa communicator alltogether. the import there is the same, i.e., import pyvisa. Probably makes not much sense there either I'd assume...
There was a problem hiding this comment.
Its an old code block from way back when it was an optional dep, because installation required a separate non-Python package to be installed.
| @@ -118,7 +115,7 @@ def read_raw(self, size=-1): | |||
|
|
|||
| elif size == -1: | |||
| # Read the whole contents, appending the buffer we've already read. | |||
| msg = self._buf + self._conn.read() | |||
| msg = self._buf + self._conn.read_raw() | |||
There was a problem hiding this comment.
BF as discussed in #307
| @@ -136,10 +133,10 @@ def write_raw(self, msg): | |||
| self._conn.write_raw(msg) | |||
|
|
|||
| def seek(self, offset): # pylint: disable=unused-argument,no-self-use | |||
| return NotImplemented | |||
| raise NotImplementedError | |||
There was a problem hiding this comment.
Make consistent with the rest of IK, same for second replacement below.
Figured I'll take care of issue #307 and incorporate a few tests for the visa communicator. Using
pyvisa-simto simulate a non-connected device.More details given with code comments. Would be great if @thestumbler could test the BF with the actual instrument present!