fix(base): fix bugs found in the printline/trprint/qprint rewrite

Fixes discovered while packaging up old WIP work (commit 9dc38f9,
"Consistent operation of printline()...") for review. Three separate
bugs, all in that same unreleased WIP, none of it previously merged
or published:

1. trprint()/qprint() silently shifted `file` into the 2nd positional
   argument slot (`def trprint(T, file=False, **kwargs)`), when the
   original, published, external-user-facing signatures had `orient`/
   `delim` there instead. `trprint(T, 'angvec')` or the library's own
   documented `x.printline('angvec')` example put 'angvec' into `file`
   instead of `orient`, crashing with
   `AttributeError: 'str' object has no attribute 'write'`.
   Fixed by restoring the full original explicit parameter order for
   trprint/qprint (trprint2 was already fine here - its parameter
   order happened to be unaffected).

2. trprint2() captured `label` as an explicit parameter but its body
   never forwarded it to tr2str2() - `trprint2(T, label='T')` silently
   dropped the label from the output.

3. All three (trprint/trprint2/qprint) stopped returning the compact
   string entirely, only printing it - breaking any caller using the
   deprecated-but-still-supported `file=None` -> return string instead
   of printing usage, which several existing tests rely on
   (test_transforms3d.py::test_print, test_transforms2d.py::test_print2).

While fixing (3): the `file=None` "give me a string instead" pattern
is a wart worth phasing out rather than perpetuating silently - each
of these now has a real *2str()/q2str() sibling that does that
directly. So `file=None` is kept working (still returns the string,
still doesn't print) but now consistently raises a DeprecationWarning
and says so explicitly in the docstring via `.. deprecated::`,
pointing at tr2str()/tr2str2()/q2str() as the replacement.

All three now follow one consistent shape: always build the string via
the *2str()/q2str() sibling, always return it, print only when
file is not None (file=False, the default, resolves to None passed to
print() so contextlib.redirect_stdout() sees the current sys.stdout at
call time rather than whatever was captured when the function was
defined - the actual bug this WIP originally set out to fix).

Verified: file=None still returns the correct string with a
DeprecationWarning and no printing; the default path both prints
(confirmed via contextlib.redirect_stdout) and returns the same
string; full test suite green (338 passed, 4 skipped);
black --check clean at 23.10.0.

Author: petercorke

spatialmath/base/quaternions.py

@@ -1152,7 +1152,12 @@ def q2str(
     return template.format(q[0], delim[0], q[1], q[2], q[3], delim[1])
 
 
-def qprint(q: Union[ArrayLike4, ArrayLike4], file=False, **kwargs) -> None:
+def qprint(
+    q: Union[ArrayLike4, ArrayLike4],
+    delim: Optional[Tuple[str, str]] = ("<", ">"),
+    fmt: Optional[str] = "{: .4f}",
+    file: Optional[TextIO] = False,
+) -> str:
     """
     Compact single-line display of a quaternion
 
@@ -1182,17 +1187,26 @@ def qprint(q: Union[ArrayLike4, ArrayLike4], file=False, **kwargs) -> None:
         >>> q = qrand()   # a unit quaternion
         >>> qprint(q, delim=('<<', '>>'))
 
+    .. deprecated:: 1.1.15
+        ``file=None`` to get the string back without printing is
+        deprecated - call :func:`~q2str` directly instead.
+
     :seealso: :meth:`q2str`
     """
     q = smb.getvector(q, 4)
+    s = q2str(q, delim=delim, fmt=fmt)
     if file is None:
         warnings.warn(
             "Usage: qprint(..., file=None) -> str is deprecated, use q2str() instead",
             DeprecationWarning,
         )
-    if file is False:
-        file = None  # defaults to stdout
-    print(q2str(q, **kwargs), file=file)
+    else:
+        # file=False (the default) resolves to None here so print() looks
+        # up the *current* sys.stdout at call time, not whatever it was
+        # when this function was defined - that's what makes
+        # contextlib.redirect_stdout() work.
+        print(s, file=None if file is False else file)
+    return s
 
 
 if __name__ == "__main__":  # pragma: no cover

spatialmath/base/transforms2d.py

@@ -963,7 +963,6 @@ def trinterp2(start, end, s, shortest: bool = True):
 def tr2str2(
     T: Union[SO2Array, SE2Array],
     label: str = "",
-    file: TextIO = None,
     fmt: str = "{:.3g}",
     unit: str = "deg",
 ) -> str:
@@ -974,8 +973,6 @@ def tr2str2(
     :type T: ndarray(3,3) or ndarray(2,2)
     :param label: text label to put at start of line
     :type label: str
-    :param file: file to write formatted string to
-    :type file: file object
     :param fmt: conversion format for each number
     :type fmt: str
     :param unit: angular units: 'rad' [default], or 'deg'
@@ -1101,19 +1098,25 @@ def trprint2(
         - For tabular data set ``fmt`` to a fixed width format such as
           ``fmt='{:.3g}'``
 
-    .. versionchanged:: 1.1.15
-        To create a string use :func:`~tr2str2` instead of ``trprint2(...file=None)``
+    .. deprecated:: 1.1.15
+        ``file=None`` to get the string back without printing is
+        deprecated - call :func:`~tr2str2` directly instead.
 
     :seealso: :func:`~tr2str2` :func:`~trprint`
     """
+    s = tr2str2(T, label=label, **kwargs)
     if file is None:
         warnings.warn(
             "Usage: trprint2(..., file=None) -> str is deprecated, use tr2str2() instead",
             DeprecationWarning,
         )
-    if file is False:
-        file = None  # defaults to stdout
-    print(tr2str2(T, **kwargs), file=file)
+    else:
+        # file=False (the default) resolves to None here so print() looks
+        # up the *current* sys.stdout at call time, not whatever it was
+        # when this function was defined - that's what makes
+        # contextlib.redirect_stdout() work.
+        print(s, file=None if file is False else file)
+    return s
 
 
 def points2tr2(p1: NDArray, p2: NDArray) -> SE2Array:

spatialmath/base/transforms3d.py

@@ -2785,7 +2785,6 @@ def tr2str(
     T: Union[SO3Array, SE3Array],
     orient: str = "rpy/zyx",
     label: str = "",
-    file: TextIO = None,
     fmt: str = "{:.3g}",
     degsym: bool = True,
     unit: str = "deg",
@@ -2904,7 +2903,15 @@ def _vec2s(fmt, v):
     return ", ".join([fmt.format(x) for x in v])
 
 
-def trprint(T: Union[SO3Array, SE3Array], file=False, **kwargs) -> str:
+def trprint(
+    T: Union[SO3Array, SE3Array],
+    orient: str = "rpy/zyx",
+    label: str = "",
+    file: TextIO = False,
+    fmt: str = "{:.3g}",
+    degsym: bool = True,
+    unit: str = "deg",
+) -> str:
     """
      Compact single-line display of SO(3) or SE(3) matrices
 
@@ -2962,20 +2969,28 @@ def trprint(T: Union[SO3Array, SE3Array], file=False, **kwargs) -> str:
          - For tabular data set ``fmt`` to a fixed width format such as
            ``fmt='{:.3g}'``
 
-    .. versionchanged:: 1.1.15
-        To create a string use :func:`~tr2str` instead of ``trprint(...file=None)``
+    .. deprecated:: 1.1.15
+        ``file=None`` to get the string back without printing is
+        deprecated - call :func:`~tr2str` directly instead.
 
      :seealso: :func:`~tr2str` :func:`~spatialmath.base.transforms2d.trprint2` :func:`~tr2eul` :func:`~tr2rpy` :func:`~tr2angvec`
      :SymPy: not supported
     """
+    s = tr2str(T, orient=orient, label=label, fmt=fmt, degsym=degsym, unit=unit)
     if file is None:
+        # deprecated: return the string without printing, matching the
+        # original `if file: print(...)` falsy-skip behaviour
         warnings.warn(
             "Usage: trprint(..., file=None) -> str is deprecated, use tr2str() instead",
             DeprecationWarning,
         )
-    if file is False:
-        file = None  # defaults to stdout
-    print(tr2str(T, **kwargs), file=file)
+    else:
+        # file=False (the default) resolves to None here so print() looks
+        # up the *current* sys.stdout at call time, not whatever it was
+        # when this function was defined - that's what makes
+        # contextlib.redirect_stdout() work.
+        print(s, file=None if file is False else file)
+    return s
 
 
 try: