8000 Merge pull request #1121 from murrayrm/fix_bode_margins_title-02Feb2025 · python-control/python-control@f73e893 · GitHub
[go: up one dir, main page]
Skip to content

Commit f73e893

Browse files
murrayrmmurrayrm
authored
Merge pull request #1121 from murrayrm/fix_bode_margins_title-02Feb2025
Fix missing plot title in bode_plot() with display_margins
2 parents 9fb84cb + 0b12203 commit f73e893

File tree

2 files changed

+86
-19
lines changed

2 files changed

+86
-19
lines changed

‎control/freqplot.py‎

Lines changed: 29 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,8 @@ def bode_plot(
134134
If True, draw gain and phase margin lines on the magnitude and phase
135135
graphs and display the margins at the top of the graph. If set to
136136
'overlay', the values for the gain and phase margin are placed on
137-
the graph. Setting display_margins turns off the axes grid.
137+
the graph. Setting `display_margins` turns off the axes grid, unless
138+
`grid` is explicitly set to True.
138139
**kwargs : `matplotlib.pyplot.plot` keyword properties, optional
139140
Additional keywords passed to `matplotlib` to specify line properties.
140141
@@ -276,6 +277,24 @@ def bode_plot(
276277
# Make a copy of the kwargs dictionary since we will modify it
277278
kwargs = dict(kwargs)
278279

280+
# Legacy keywords for margins
281+
display_margins = config._process_legacy_keyword(
282+
kwargs, 'margins', 'display_margins', display_margins)
283+
if kwargs.pop('margin_info', False):
284+
warnings.warn(
285+
"keyword 'margin_info' is deprecated; "
286+
"use 'display_margins='overlay'")
287+
if display_margins is False:
288+
raise ValueError(
289+
"conflicting_keywords: `display_margins` and `margin_info`")
290+
291+
# Turn off grid if display margins, unless explicitly overridden
292+
if display_margins and 'grid' not in kwargs:
293+
kwargs['grid'] = False
294+
295+
margins_method = config._process_legacy_keyword(
296+
kwargs, 'method', 'margins_method', margins_method)
297+
279298
# Get values for params (and pop from list to allow keyword use in plot)
280299
dB = config._get_param(
281300
'freqplot', 'dB', kwargs, _freqplot_defaults, pop=True)
@@ -316,19 +335,6 @@ def bode_plot(
316335
"sharex cannot be present with share_frequency")
317336
kwargs['share_frequency'] = sharex
318337

319-
# Legacy keywords for margins
320-
display_margins = config._process_legacy_keyword(
321-
kwargs, 'margins', 'display_margins', display_margins)
322-
if kwargs.pop('margin_info', False):
323-
warnings.warn(
324-
"keyword 'margin_info' is deprecated; "
325-
"use 'display_margins='overlay'")
326-
if display_margins is False:
327-
raise ValueError(
328-
"conflicting_keywords: `display_margins` and `margin_info`")
329-
margins_method = config._process_legacy_keyword(
330-
kwargs, 'method', 'margins_method', margins_method)
331-
332338
if not isinstance(data, (list, tuple)):
333339
data = [data]
334340

@@ -727,7 +733,7 @@ def _make_line_label(response, output_index, input_index):
727733
label='_nyq_mag_' + sysname)
728734

729735
# Add a grid to the plot
730-
ax_mag.grid(grid and not display_margins, which='both')
736+
ax_mag.grid(grid, which='both')
731737

732738
# Phase
733739
if plot_phase:
@@ -742,7 +748,7 @@ def _make_line_label(response, output_index, input_index):
742748
label='_nyq_phase_' + sysname)
743749

744750
# Add a grid to the plot
745-
ax_phase.grid(grid and not display_margins, which='both')
751+
ax_phase.grid(grid, which='both')
746752

747753
#
748754
# Display gain and phase margins (SISO only)
@@ -753,6 +759,10 @@ def _make_line_label(response, output_index, input_index):
753759
raise NotImplementedError(
754760
"margins are not available for MIMO systems")
755761

762+
if display_margins == 'overlay' and len(data) > 1:
763+
raise NotImplementedError(
764+
f"{display_margins=} not supported for multi-trace plots")
765+
756766
# Compute stability margins for the system
757767
margins = stability_margins(response, method=margins_method)
758768
gm, pm, Wcg, Wcp = (margins[i] for i in [0, 1, 3, 4])
@@ -844,12 +854,12 @@ def _make_line_label(response, output_index, input_index):
844854

845855
else:
846856
# Put the title underneath the suptitle (one line per system)
847-
ax = ax_mag if ax_mag else ax_phase
848-
axes_title = ax.get_title()
857+
ax_ = ax_mag if ax_mag else ax_phase
858+
axes_title = ax_.get_title()
849859
if axes_title is not None and axes_title != "":
850860
axes_title += "\n"
851861
with plt.rc_context(rcParams):
852-
ax.set_title(
862+
ax_.set_title(
853863
axes_title + f"{sysname}: "
854864
"Gm = %.2f %s(at %.2f %s), "
855865
"Pm = %.2f %s (at %.2f %s)" %

‎control/tests/freqplot_test.py‎

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
# freqplot_test.py - test out frequency response plots
22
# RMM, 23 Jun 2023
33

4+
import re
45
import matplotlib as mpl
56
import matplotlib.pyplot as plt
67
import numpy as np
@@ -597,6 +598,12 @@ def test_suptitle():
597598
TypeError, match="unexpected keyword|no property"):
598599
cplt.set_plot_title("New title", unknown=None)
599600

601+
# Make sure title is still there if we display margins underneath
602+
sys = ct.rss(2, 1, 1, name='sys')
603+
cplt = ct.bode_plot(sys, display_margins=True)
604+
assert re.match(r"^Bode plot for sys$", cplt.figure._suptitle._text)
605+
assert re.match(r"^sys: Gm = .*, Pm = .*$", cplt.axes[0, 0].get_title())
606+
600607

601608
@pytest.mark.parametrize("plt_fcn", [ct.bode_plot, ct.singular_values_plot])
602609
def test_freqplot_errors(plt_fcn):
@@ -617,6 +624,7 @@ def test_freqplot_errors(plt_fcn):
617624
with pytest.raises(ValueError, match="invalid limits"):
618625
plt_fcn(response, omega_limits=[1e2, 1e-2])
619626

627+
620628
def test_freqresplist_unknown_kw():
621629
sys1 = ct.rss(2, 1, 1)
622630
sys2 = ct.rss(2, 1, 1)
@@ -626,6 +634,52 @@ def test_freqresplist_unknown_kw():
626634
with pytest.raises(AttributeError, match="unexpected keyword"):
627635
resp.plot(unknown=True)
628636

637+
@pytest.mark.parametrize("nsys, display_margins, gridkw, match", [
638+
(1, True, {}, None),
639+
(1, False, {}, None),
640+
(1, False, {}, None),
641+
(1, True, {'grid': True}, None),
642+
(1, 'overlay', {}, None),
643+
(1, 'overlay', {'grid': True}, None),
644+
(1, 'overlay', {'grid': False}, None),
645+
(2, True, {}, None),
646+
(2, 'overlay', {}, < A36C span class=pl-s>"not supported for multi-trace plots"),
647+
(2, True, {'grid': 'overlay'}, None),
648+
(3, True, {'grid': True}, None),
649+
])
650+
def test_display_margins(nsys, display_margins, gridkw, match):
651+
sys1 = ct.tf([10], [1, 1, 1, 1], name='sys1')
652+
sys2 = ct.tf([20], [2, 2, 2, 1], name='sys2')
653+
sys3 = ct.tf([30], [2, 3, 3, 1], name='sys3')
654+
655+
sysdata = [sys1, sys2, sys3][0:nsys]
656+
657+
plt.figure()
658+
if match is None:
659+
cplt = ct.bode_plot(sysdata, display_margins=display_margins, **gridkw)
660+
else:
661+
with pytest.raises(NotImplementedError, match=match):
662+
ct.bode_plot(sysdata, display_margins=display_margins, **gridkw)
663+
return
664+
665+
cplt.set_plot_title(
666+
cplt.figure._suptitle._text + f" [d_m={display_margins}, {gridkw=}")
667+
668+
# Make sure the grid is there if it should be
669+
if gridkw.get('grid') or not display_margins:
670+
assert all(
671+
[line.get_visible() for line in cplt.axes[0, 0].get_xgridlines()])
672+
else:
673+
assert not any(
674+
[line.get_visible() for line in cplt.axes[0, 0].get_xgridlines()])
675+
676+
# Make sure margins are displayed
677+
if display_margins == True:
678+
ax_title = cplt.axes[0, 0].get_title()
679+
assert len(ax_title.split('\n')) == nsys
680+
elif display_margins == 'overlay':
681+
assert cplt.axes[0, 0].get_title() == ''
682+
629683

630684
if __name__ == "__main__":
631685
#
@@ -680,3 +734,6 @@ def test_freqresplist_unknown_kw():
680734
# of them for use in the documentation).
681735
#
682736
test_mixed_systypes()
737+
test_display_margins(2, True, {})
738+
test_display_margins(2, 'overlay', {})
739+
test_display_margins(2, True, {'grid': True})

0 commit comments

Comments
 (0)
0