Fix support for dialogs without response

[dougthor42]

Fixes #76.

Re-add support for missing "response" key in the dialogues part and add tests for _get_pair and _get_triplet so that a similar regression won't happen in the future.

Note: Fully fixing the type annotations ended up being a much larger endeavor than expected, so I left that out of this PR and simply used type: ignore.

[MatthieuDartiailh]

Out of curiosity, why can't you apply the same typing you use in the tests ?

[dougthor42]

TL;DR: You can see what all the type changes are in this branch: https://github.com/dougthor42/pyvisa-sim/tree/gh-76-fix-dialog-wo-response-types

Changing the return type of _get_pair from str to OptionalStr ends up cascading down to:

• channels.Channels.add_dialogue
• channels.Channels._dialogues
• channels.Channels._getters
• component.Component.add_dialogue
• component.Component._dialogues
• component.Component._getters
• component.Component._match_dialog
  • which then requires some type guarding if response is NoResponse:
• devices.Device.add_eom
• devices.Device._response_eom
• devices.Device._eoms

After doing all of that, you get to:

$ mypy pyvisa_sim/
pyvisa_sim/devices.py:258: error: Argument 1 to "extend" of "bytearray" has incompatible type "Union[bytes, Literal[Responses.NO]]"; expected "Iterable[SupportsIndex]"  [arg-type]

pyvisa-sim/pyvisa_sim/devices.py

Lines 256 to 258 in 6fb7d9d

	if response is not NoResponse:
	self._output_buffer.extend(response)
	self._output_buffer.extend(eom)

Which requires it's own type guard if eom is not NoResponse:

                 if response is not NoResponse:
                     self._output_buffer.extend(response)
-                    self._output_buffer.extend(eom)
+                    if eom is not NoResponse:
+                       self._output_buffer.extend(eom)

I'm not yet familiar enough with the code to know if the type guard should be on both self._output_buffer (if response is not NoResponse and eom is not NoResponse) or just on one (like in the diff above). More testing and investigation is required.

In the end I didn't feel it was (a) worth it right now and (b) suitable to fix in this PR.

[codecov-commenter]

Codecov Report

Merging #78 (a15b834) into main (6fb7d9d) will increase coverage by 0.28%.
The diff coverage is 100.00%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@            Coverage Diff             @@
##             main      #78      +/-   ##
==========================================
+ Coverage   82.52%   82.81%   +0.28%     
==========================================
  Files          15       16       +1     
  Lines        1076     1094      +18     
  Branches      167      171       +4     
==========================================
+ Hits          888      906      +18     
  Misses        142      142              
  Partials       46       46              

Flag	Coverage Δ
unittests	82.81% <100.00%> (+0.28%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
pyvisa_sim/parser.py	73.91% <100.00%> (ø)
pyvisa_sim/testsuite/test_parser.py	100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[MatthieuDartiailh]

Thanks. Type hints are a recent addition and I did not get them all right. I will merge as is but feel free to make a PR regarding teh type annotations and I will have a look.