Merge #525

525: Allow extensions to use the format pyvisa_name r=MatthieuDartiailh a=MatthieuDartiailh

This change is motivated pyvisa/pyvisa-py#239 but only implement the simplest fix. Moving towards setuptools entry points is left to a future PR.
Additional changes include:
- a convenience method on BaseVisaLibrary to handle status code in a uniform manner in alternative backends
- a transition towards dataclasses for resource name. This change makes the code more readable and more amenable to typing. One can still get a tuple if necessary using astuple but this should not cause any major breakage.
- add a PR template to inform people about formatting tools.

Co-authored-by: MatthieuDartiailh <marul@laposte.net>

.github/PULL_REQUEST_TEMPLATE.md

@@ -0,0 +1,33 @@
+<!--
+
+Thanks for wanting to contribute to PyVISA :)
+
+Here's some guidelines to help the review process go smoothly.
+
+1. Please write a description in this text box of the changes that are being
+   made.
+
+2. Please ensure that the code is properly formatted and typed by running
+   black, isort, flake8 and mypy. You can also use pre-commit hooks (see the
+   developer documentation for detailed instructions)
+
+3. Please ensure that you have written units tests for the changes made/features
+   added.
+
+4. If you are closing an issue please use one of the automatic closing words as
+   noted here: https://help.github.com/articles/closing-issues-using-keywords/
+
+5. Once review has taken place please do not add features or make changes out of
+   the scope of those requested by the reviewer (doing this just add delays as
+   already reviewed code ends up having to be re-reviewed/it is hard to tell
+   what is new etc!).
+
+Many thanks in advance for your cooperation!
+
+-->
+
+- [ ] Closes # (insert issue number if relevant)
+- [ ] Executed ``black . && isort -c . && flake8`` with no errors
+- [ ] The change is fully covered by automated unit tests
+- [ ] Documented in docs/ as appropriate
+- [ ] Added an entry to the CHANGES file

.github/workflows/ci.yml

@@ -33,7 +33,7 @@ jobs:
           pip install flake8 black isort mypy
       - name: Isort
         run: |
-          isort pyvisa -rc -c;
+          isort pyvisa -c;
       - name: Black
         run: |
           black pyvisa --check;

CHANGES

@@ -14,15 +14,20 @@ to the old behavior by setting the environment variable PYVISA_WRAP_HANDLER=0 or
 set ctwrapper.WRAP_HANDLER to False but please consider updating to the new
 behavior.
 
-- Add Event class to provide a nice interface to VISA events PR #
-- Add `wrap_handler` to provide a nicer way to write event handler PR #
-- Add wrapper classes for events PR #
-- Add typing to the entire codebase PR #
-- Use black and isort on the code to homogenize style PR #
-- Convert docstrings to use numpy formatting PR #
-- Explicitly set attributes on resources to make teh code much more readable
+- Provide VisaLibraryBase.handle_return_value to ease the handling of return values
+  in alternative backends PR #525
+- Transition to using dataclasses for resource name PR #525
+  This is NOT fully backward compatible if you used to index the resource name
+- Allow alternative backends to use an _ instead of a - in their name PR #525
+- Add Event class to provide a nice interface to VISA events PR #511
+- Add `wrap_handler` to provide a nicer way to write event handler PR #511
+- Add wrapper classes for events PR #511
+- Add typing to the entire codebase PR #511
+- Use black and isort on the code to homogenize style PR #511
+- Convert docstrings to use numpy formatting PR #511
+- Explicitly set attributes on resources to make the code more readable PR #511
 - Make MessageBasedResource.read_bytes break on message end when
-  `break_on_termchar` is True PR #
+  `break_on_termchar` is True PR #511
 - Add support for dll_extra_paths in .pyvisarc to provide a way to specify paths
   in which to look for dll on Windows and Python >= 3.8  PR #509
 - Drop Python 2 support PR #486

docs/source/advanced/architecture.rst

@@ -23,6 +23,8 @@ PyVISA implements convenient and Pythonic programming in three layers:
         backends. For broader compatibility, do no use this layer. All the
         functionality should is available via the next layer.
 
+        Alternative backends have no obligation to provide those functions.
+
 
  2. Middle-level: A wrapping Python function for each function of the shared
     visa library.

docs/source/advanced/backends.rst

@@ -45,7 +45,7 @@ PyVISA locates backends by name. If you do:
     >>> import pyvisa
     >>> rm = pyvisa.ResourceManager('@somename')
 
-PyVISA will try to import a package/module named ``pyvisa-somename`` which
+PyVISA will try to import a package/module named ``pyvisa_somename`` which
 should be installed in your system. This is a loosely coupled configuration
 free method. PyVISA does not need to know about any backend out there until you
 actually try to use it.
@@ -72,13 +72,6 @@ Additionally you can provide a staticmethod named `get_debug_info` that should
 return a dictionary of debug information which is printed when you call
 ``pyvisa-info``
 
-.. note::
-
-    Your backend name should not end by ``-script`` or it will be discarded.
-    This is because any script generated by setuptools containing the name
-    pyvisa will be named ``pyvisa-*-script`` and they are obviously not
-    backends. Examples are the ``pyvisa-shell`` and ``pyvisa-info`` scripts.
-
 An important aspect of developing a backend is knowing which VisaLibraryBase
 method to implement and what API to expose.
 
@@ -114,7 +107,10 @@ For other usages or devices, you might need to implement other functions. Is
 really up to you and your needs.
 
 These functions should raise a :class:`pyvisa.errors.VisaIOError` or emit a
-:class:`pyvisa.errors.VisaIOWarning` if necessary.
+:class:`pyvisa.errors.VisaIOWarning` if necessary, and store error code on a
+per session basis. This can be done easily by calling
+:py:meth:`~pyvisa.highlevel.VisaLibraryBase.handle_return_value` with the session
+and return value.
 
 
 Complete list of level 2 functions to implement::
@@ -198,4 +194,3 @@ Complete list of level 2 functions to implement::
     def write(self, session, data):
     def write_asynchronously(self, session, data):
     def write_from_file(self, session, filename, count):
-

pyvisa/ctwrapper/cthelper.py

@@ -22,9 +22,9 @@
 
     # Andreas Degert's find functions, using gcc, /sbin/ldconfig, objdump
     def define_find_libary():
+        import errno
         import re
         import tempfile
-        import errno
 
         def _findlib_gcc(name):
             expr = r"[^\(\)\s]*lib%s\.[^\(\)\s]*" % re.escape(name)

pyvisa/ctwrapper/highlevel.py

@@ -22,7 +22,6 @@
     Type,
     TypeVar,
     Union,
-    cast,
 )
 
 from pyvisa import constants, errors, highlevel, logger, typing
@@ -188,14 +187,6 @@ def _return_handler(self, ret_value: int, func: Callable, arguments: tuple) -> A
             extra=self._logging_extra,
         )
 
-        rv: constants.StatusCode
-        try:
-            rv = constants.StatusCode(ret_value)
-        except ValueError:
-            rv = cast(constants.StatusCode, ret_value)
-
-        self._last_status = rv
-
         # The first argument of almost all registered visa functions is a session.
         # We store the error code per session
         session = None
@@ -210,13 +201,10 @@ def _return_handler(self, ret_value: int, func: Callable, arguments: tuple) -> A
 
             # Functions that use the first parameter to get a session value.
             if func.__name__ in ("viOpenDefaultRM",):
-                # noinspection PyProtectedMember
                 session = session._obj.value
 
-            if isinstance(session, int):
-                self._last_status_in_session[session] = rv
-            else:
-                # Functions that might or might have a session in the first argument.
+            if not isinstance(session, int):
+                # Functions that might or might not have a session in the first argument.
                 if func.__name__ not in (
                     "viClose",
                     "viGetAttribute",
@@ -228,14 +216,10 @@ def _return_handler(self, ret_value: int, func: Callable, arguments: tuple) -> A
                         "visa function (type args[0] %r)" % (func, type(session))
                     )
 
-        if ret_value < 0:
-            raise errors.VisaIOError(rv)
-
-        if rv in self.issue_warning_on:
-            if session and ret_value not in self._ignore_warning_in_session[session]:
-                warnings.warn(errors.VisaIOWarning(ret_value), stacklevel=2)
+                # Set session back to a safe value
+                session = None
 
-        return ret_value
+        return self.handle_return_value(session, ret_value)  # type: ignore
 
     def list_resources(
         self, session: typing.VISARMSession, query: str = "?*::INSTR"

pyvisa/highlevel.py

@@ -41,6 +41,7 @@
 from typing_extensions import ClassVar, DefaultDict, Literal
 
 from . import attributes, constants, errors, logger, rname
+from .constants import StatusCode
 from .typing import (
     VISAEventContext,
     VISAHandler,
@@ -117,7 +118,9 @@ class VisaLibraryBase(object):
     _last_status: constants.StatusCode = constants.StatusCode(0)
 
     #: Maps session handle to last status.
-    _last_status_in_session: Dict[int, constants.StatusCode]
+    _last_status_in_session: Dict[
+        Union[VISASession, VISARMSession, VISAEventContext], constants.StatusCode
+    ]
 
     #: Maps session handle to warnings to ignore.
     _ignore_warning_in_session: Dict[int, set]
@@ -221,6 +224,37 @@ def __repr__(self) -> str:
         """str representation of the library including the type of the library."""
         return "<%s(%r)>" % (type(self).__name__, self.library_path)
 
+    def handle_return_value(
+        self,
+        session: Optional[Union[VISAEventContext, VISARMSession, VISASession]],
+        status_code: int,
+    ) -> StatusCode:
+        """Helper function handling the return code of a low-level operation.
+
+        Used when implementing concrete subclasses of VISALibraryBase.
+
+        """
+        rv: constants.StatusCode
+        try:
+            rv = constants.StatusCode(status_code)
+        except ValueError:
+            rv = cast(constants.StatusCode, status_code)
+
+        self._last_status = rv
+
+        if session is not None:
+            self._last_status_in_session[session] = rv
+
+        if rv < 0:
+            raise errors.VisaIOError(rv)
+
+        if rv in self.issue_warning_on:
+            if session and rv not in self._ignore_warning_in_session[session]:
+                warnings.warn(errors.VisaIOWarning(rv), stacklevel=2)
+
+        # Return the original value for further processing.
+        return rv
+
     @property
     def last_status(self) -> constants.StatusCode:
         """Last return value of the library."""
@@ -2014,7 +2048,13 @@ def parse_resource_extended(
             return (
                 ResourceInfo(
                     parsed.interface_type_const,
-                    int(parsed.board),  # match IVI-VISA
+                    # We can only get concrete classes which have one of those
+                    # attributes
+                    int(
+                        parsed.board  # type: ignore
+                        if hasattr(parsed, "board")
+                        else parsed.interface  # type: ignore
+                    ),
                     parsed.resource_class,
                     str(parsed),
                     None,
@@ -2790,12 +2830,26 @@ def get_wrapper_class(backend_name: str) -> Type[VisaLibraryBase]:
                 )
             return IVIVisaLibrary
 
+    pkg: PyVISAModule
     try:
-        pkg: PyVISAModule = cast(PyVISAModule, import_module("pyvisa-" + backend_name))
+        pkg = cast(PyVISAModule, import_module("pyvisa_" + backend_name))
         _WRAPPERS[backend_name] = cls = pkg.WRAPPER_CLASS
         return cls
     except ImportError:
-        raise ValueError("Wrapper not found: No package named pyvisa-%s" % backend_name)
+        try:
+            pkg = cast(PyVISAModule, import_module("pyvisa-" + backend_name))
+            _WRAPPERS[backend_name] = cls = pkg.WRAPPER_CLASS
+            warnings.warn(
+                "Backends packages should use an _ rather than a - ."
+                "Project can/should keep using a - (like pytest plugins)."
+                "Support for backends with - will be removed in 1.12",
+                FutureWarning,
+            )
+            return cls
+        except ImportError:
+            raise ValueError(
+                "Wrapper not found: No package named pyvisa_%s" % backend_name
+            )
 
 
 def _get_default_wrapper() -> str: