update plot handling to allow system arguments

murrayrm · murrayrm · commit d07422db2bfb · 2023-09-16T09:49:26.000-07:00
diff --git a/control/freqplot.py b/control/freqplot.py
@@ -201,8 +201,10 @@ def bode_plot(
 
     Other Parameters
     ----------------
-    plot : bool
-        If True (default), plot magnitude and phase.
+    plot : bool, optional
+        (legacy) If given, `bode_plot` returns the legacy return values
+        of magnitude, phase, and frequency.  If False, just return the
+        values with no plot.
     omega_limits : array_like of two values
         Limits of the to generate frequency vector. If Hz=True the limits
         are in Hz otherwise in rad/s.
@@ -232,22 +234,20 @@ def bode_plot(
 
     Notes
     -----
-    1. Alternatively, you may use the lower-level methods
-       :meth:`LTI.frequency_response` or ``sys(s)`` or ``sys(z)`` or to
-       generate the frequency response for a single system.
+    1. Starting with python-control version 0.10, `bode_plot`returns an
+       array of lines instead of magnitude, phase, and frequency. To
+       recover the # old behavior, call `bode_plot` with `plot=True`, which
+       will force the legacy return values to be used (with a warning).  To
+       obtain just the frequency response of a system (or list of systems)
+       without plotting, use the :func:`~control.frequency_response`
+       command.
 
     2. If a discrete time model is given, the frequency response is plotted
        along the upper branch of the unit circle, using the mapping ``z =
        exp(1j * omega * dt)`` where `omega` ranges from 0 to `pi/dt` and `dt`
        is the discrete timebase.  If timebase not specified (``dt=True``),
        `dt` is set to 1.
 
-    3. The legacy version of this function is invoked if instead of passing
-       frequency response data, a system (or list of systems) is passed as
-       the first argument, or if the (deprecated) keyword `plot` is set to
-       True or False. The return value is then given as `mag`, `phase`,
-       `omega` for the plotted frequency response (SISO only).
-
     Examples
     --------
     >>> G = ct.ss([[-1, -2], [3, -4]], [[5], [7]], [[6, 8]], [[9]])
@@ -315,18 +315,6 @@ def bode_plot(
     if not isinstance(data, (list, tuple)):
         data = [data]
 
-    # For backwards compatibility, allow systems in the data list
-    if all([isinstance(
-            sys, (StateSpace, TransferFunction)) for sys in data]):
-        data = frequency_response(
-            data, omega=omega, omega_limits=omega_limits,
-            omega_num=omega_num)
-        warnings.warn(
-            "passing systems to `bode_plot` is deprecated; "
-            "use `frequency_response()`", DeprecationWarning)
-        if plot is None:
-            plot = True         # Keep track of legacy usage (see notes below)
-
     #
     # Pre-process the data to be plotted (unwrap phase)
     #
@@ -337,6 +325,13 @@ def bode_plot(
     # the list of lines created, which is the new output for _plot functions.
     #
 
+    # If we were passed a list of systems, convert to data
+    if all([isinstance(
+            sys, (StateSpace, TransferFunction)) for sys in data]):
+        data = frequency_response(
+            data, omega=omega, omega_limits=omega_limits,
+            omega_num=omega_num, Hz=Hz)
+
     # If plot_phase is not specified, check the data first, otherwise true
     if plot_phase is None:
         plot_phase = True if data[0].plot_phase is None else data[0].plot_phase
@@ -409,28 +404,25 @@ def bode_plot(
     #
     # There are three possibilities at this stage in the code:
     #
-    # * plot == True: either set explicitly by the user or we were passed a
-    #   non-FRD system instead of data.  Return mag, phase, omega, with a
-    #   warning.
+    # * plot == True: set explicitly by the user. Return mag, phase, omega,
+    #   with a warning.
     #
     # * plot == False: set explicitly by the user. Return mag, phase,
     #   omega, with a warning.
     #
-    # * plot == None: this is the new default setting and if it hasn't been
-    #   changed, then we use the v0.10+ standard of returning an array of
+    # * plot == None: this is the new default setting.  Return an array of
     #   lines that were drawn.
     #
-    # The one case that can cause problems is that a user called
-    # `bode_plot` with an FRD system, didn't set the plot keyword
-    # explicitly, and expected mag, phase, omega as a return value.  This
-    # is hopefully a rare case (it wasn't in any of our unit tests nor
-    # examples at the time of v0.10.0).
+    # If `bode_plot` was called with no `plot` argument and the return
+    # values were used, the new code will cause problems (you get an array
+    # of lines instead of magnitude, phase, and frequency).  To recover the
+    # old behavior, call `bode_plot` with `plot=True`.
     #
     # All of this should be removed in v0.11+ when we get rid of deprecated
     # code.
     #
 
-    if plot is True or plot is False:
+    if plot is not None:
         warnings.warn(
             "`bode_plot` return values of mag, phase, omega is deprecated; "
             "use frequency_response()", DeprecationWarning)
@@ -1828,8 +1820,8 @@ def _compute_curve_offset(resp, mask, max_offset):
 def gangof4_response(P, C, omega=None, Hz=False):
     """Compute the response of the "Gang of 4" transfer functions for a system.
 
-    Generates a 2x2 plot showing the "Gang of 4" sensitivity functions
-    [T, PS; CS, S].
+    Generates a 2x2 frequency response for the "Gang of 4" sensitivity
+    functions [T, PS; CS, S].
 
     Parameters
     ----------
@@ -1840,7 +1832,9 @@ def gangof4_response(P, C, omega=None, Hz=False):
 
     Returns
     -------
-    None
+    response : :class:`~control.FrequencyResponseData`
+        Frequency response with inputs 'r' and 'd' and outputs 'y', and 'u'
+        representing the 2x2 matrix of transfer functions in the Gang of 4.
 
     Examples
     --------
@@ -1989,6 +1983,10 @@ def singular_values_plot(
     Hz : bool
         If True, plot frequency in Hz (omega must be provided in rad/sec).
         Default value (False) set by config.defaults['freqplot.Hz'].
+    plot : bool, optional
+        (legacy) If given, `singular_values_plot` returns the legacy return
+        values of magnitude, phase, and frequency.  If False, just return
+        the values with no plot.
     *fmt : :func:`matplotlib.pyplot.plot` format string, optional
         Passed to `matplotlib` as the format string for all lines in the plot.
         The `omega` parameter must be present (use omega=None if needed).
@@ -2023,28 +2021,20 @@ def singular_values_plot(
     freqplot_rcParams = config._get_param(
         'freqplot', 'rcParams', kwargs, _freqplot_defaults, pop=True)
 
-    # Process legacy system arguments
+    # Convert systems into frequency responses
     if any([isinstance(response, (StateSpace, TransferFunction))
             for response in data]):
-        warnings.warn(
-            "passing systems to `singular_values_plot` is deprecated; "
-            "use `singular_values_response()`", DeprecationWarning)
         responses = singular_values_response(
                     data, omega=omega, omega_limits=omega_limits,
                     omega_num=omega_num)
-        legacy_usage = True
     else:
         responses = data
-        legacy_usage = False
 
     # Process (legacy) plot keyword
     if plot is not None:
         warnings.warn(
             "`singular_values_plot` return values of sigma, omega is "
             "deprecated; use singular_values_response()", DeprecationWarning)
-        legacy_usage = True
-    else:
-        plot = True
 
     # Warn the user if we got past something that is not real-valued
     if any([not np.allclose(np.imag(response.fresp[:, 0, :]), 0)
@@ -2172,7 +2162,8 @@ def singular_values_plot(
     with plt.rc_context(freqplot_rcParams):
         fig.suptitle(title)
 
-    if legacy_usage:
+    # Legacy return processing
+    if plot is not None:
         if len(responses) == 1:
             return sigmas[0], omegas[0]
         else:
diff --git a/control/lti.py b/control/lti.py
@@ -424,8 +424,11 @@ def frequency_response(
 
     Notes
     -----
-    This function is a wrapper for :meth:`StateSpace.frequency_response` and
-    :meth:`TransferFunction.frequency_response`.
+    1. This function is a wrapper for :meth:`StateSpace.frequency_response`
+       and :meth:`TransferFunction.frequency_response`.
+
+    2. You can also use the lower-level methods ``sys(s)`` or ``sys(z)`` to
+       generate the frequency response for a single system.
 
     Examples
     --------
diff --git a/control/matlab/wrappers.py b/control/matlab/wrappers.py
@@ -64,11 +64,14 @@ def bode(*args, **kwargs):
     """
     from ..freqplot import bode_plot
 
+    # Use the plot keyword to get legacy behavior
+    # TODO: update to call frequency_response and then bode_plot
+    kwargs = dict(kwargs)       # make a copy since we modify this
+    if 'plot' not in kwargs:
+        kwargs['plot'] = True
+
     # Turn off deprecation warning
     with warnings.catch_warnings():
-        warnings.filterwarnings(
-            'ignore', message='passing systems .* is deprecated',
-            category=DeprecationWarning)
         warnings.filterwarnings(
             'ignore', message='.* return values of .* is deprecated',
             category=DeprecationWarning)
diff --git a/control/tests/config_test.py b/control/tests/config_test.py
@@ -203,36 +203,42 @@ def test_custom_bode_default(self, mplcleanup):
     @pytest.mark.usefixtures("legacy_plot_signature")
     def test_bode_number_of_samples(self, mplcleanup):
         # Set the number of samples (default is 50, from np.logspace)
-        mag_ret, phase_ret, omega_ret = ct.bode_plot(self.sys, omega_num=87)
+        mag_ret, phase_ret, omega_ret = ct.bode_plot(
+            self.sys, omega_num=87, plot=True)
         assert len(mag_ret) == 87
 
         # Change the default number of samples
         ct.config.defaults['freqplot.number_of_samples'] = 76
-        mag_ret, phase_ret, omega_ret = ct.bode_plot(self.sys)
+        mag_ret, phase_ret, omega_ret = ct.bode_plot(self.sys, plot=True)
         assert len(mag_ret) == 76
 
         # Override the default number of samples
-        mag_ret, phase_ret, omega_ret = ct.bode_plot(self.sys, omega_num=87)
+        mag_ret, phase_ret, omega_ret = ct.bode_plot(
+            self.sys, omega_num=87, plot=True)
         assert len(mag_ret) == 87
 
     @pytest.mark.usefixtures("legacy_plot_signature")
     def test_bode_feature_periphery_decade(self, mplcleanup):
         # Generate a sample Bode plot to figure out the range it uses
         ct.reset_defaults()     # Make sure starting state is correct
-        mag_ret, phase_ret, omega_ret = ct.bode_plot(self.sys, Hz=False)
+        mag_ret, phase_ret, omega_ret = ct.bode_plot(
+            self.sys, Hz=False, plot=True)
         omega_min, omega_max = omega_ret[[0,  -1]]
 
         # Reset the periphery decade value (should add one decade on each end)
         ct.config.defaults['freqplot.feature_periphery_decades'] = 2
-        mag_ret, phase_ret, omega_ret = ct.bode_plot(self.sys, Hz=False)
+        mag_ret, phase_ret, omega_ret = ct.bode_plot(
+            self.sys, Hz=False, plot=True)
         np.testing.assert_almost_equal(omega_ret[0], omega_min/10)
         np.testing.assert_almost_equal(omega_ret[-1], omega_max * 10)
 
         # Make sure it also works in rad/sec, in opposite direction
-        mag_ret, phase_ret, omega_ret = ct.bode_plot(self.sys, Hz=True)
+        mag_ret, phase_ret, omega_ret = ct.bode_plot(
+            self.sys, Hz=True, plot=True)
         omega_min, omega_max = omega_ret[[0,  -1]]
         ct.config.defaults['freqplot.feature_periphery_decades'] = 1
-        mag_ret, phase_ret, omega_ret = ct.bode_plot(self.sys, Hz=True)
+        mag_ret, phase_ret, omega_ret = ct.bode_plot(
+            self.sys, Hz=True, plot=True)
         np.testing.assert_almost_equal(omega_ret[0], omega_min*10)
         np.testing.assert_almost_equal(omega_ret[-1], omega_max/10)
 
diff --git a/control/tests/discrete_test.py b/control/tests/discrete_test.py
@@ -465,7 +465,7 @@ def test_discrete_bode(self, tsys):
         # Create a simple discrete time system and check the calculation
         sys = TransferFunction([1], [1, 0.5], 1)
         omega = [1, 2, 3]
-        mag_out, phase_out, omega_out = bode(sys, omega)
+        mag_out, phase_out, omega_out = bode(sys, omega, plot=True)
         H_z = list(map(lambda w: 1./(np.exp(1.j * w) + 0.5), omega))
         np.testing.assert_array_almost_equal(omega, omega_out)
         np.testing.assert_array_almost_equal(mag_out, np.absolute(H_z))
diff --git a/control/tests/freqresp_test.py b/control/tests/freqresp_test.py
@@ -360,19 +360,20 @@ def test_options(editsdefaults):
      ])
 def test_initial_phase(TF, initial_phase, default_phase, expected_phase):
     # Check initial phase of standard transfer functions
-    mag, phase, omega = ctrl.bode(TF)
+    mag, phase, omega = ctrl.bode(TF, plot=True)
     assert(abs(phase[0] - default_phase) < 0.1)
 
     # Now reset the initial phase to +180 and see if things work
-    mag, phase, omega = ctrl.bode(TF, initial_phase=initial_phase)
+    mag, phase, omega = ctrl.bode(TF, initial_phase=initial_phase, plot=True)
     assert(abs(phase[0] - expected_phase) < 0.1)
 
     # Make sure everything works in rad/sec as well
     if initial_phase:
         plt.xscale('linear')  # avoids xlim warning on next line
         plt.clf()  # clear previous figure (speeds things up)
         mag, phase, omega = ctrl.bode(
-            TF, initial_phase=initial_phase/180. * math.pi, deg=False)
+            TF, initial_phase=initial_phase/180. * math.pi,
+            deg=False, plot=True)
         assert(abs(phase[0] - expected_phase) < 0.1)
 
 
@@ -399,7 +400,7 @@ def test_initial_phase(TF, initial_phase, default_phase, expected_phase):
                   -270, -3*math.pi/2, math.pi/2,    id="order5, -270"),
     ])
 def test_phase_wrap(TF, wrap_phase, min_phase, max_phase):
-    mag, phase, omega = ctrl.bode(TF, wrap_phase=wrap_phase)
+    mag, phase, omega = ctrl.bode(TF, wrap_phase=wrap_phase, plot=True)
     assert(min(phase) >= min_phase)
     assert(max(phase) <= max_phase)
 
