-
Notifications
You must be signed in to change notification settings - Fork 438
Control plot refactoring for consistent functionality #1034
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Control plot refactoring for consistent functionality #1034
Conversation
9a407ac
to
a9ffba5
Compare
I will review this today (Saturday, 3 Aug). |
73c8c80
to
d66f465
Compare
This is taking a bit longer; I am almost done and will submit one review (instead of many small incremental comments). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Excellent work! The consistency here is a good improvement for user/developer experience.
I made several small comments at the relevant lines and below. Ready to merge after you consider these.
If there are ruff
options in pyproject, ruff
itself should be install-able as an optional dependency. E.g., I think it fits in the test
category (with pytest
), or we can make a new category like dev
in the [project.optional-dependencies]
section. Here are examples in other popular projects for comparison:
- in Scipy
- in Altair
- counter-example: Pandas has
ruff
options but noruff
dependency in pyproject.toml
Many of the first row subplots now show horizontal axis numbers, even when these are the same and aligned with the bottom row subplots. E.g., doc/timeplot-mimo_step-default.png. I think this style is good, too, but I want to note it because there is not a commit in this PR that explicitly describes this change (so, maybe change was not intended?).
control/freqplot.py
Outdated
string (see :func:`~matplotlib.pyplot.legend`). | ||
legend_loc : int or str, optional | ||
Include a legend in the given location. Default is 'center right', | ||
with no legend for a single response. Use False to supress legend. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
with no legend for a single response. Use False to supress legend. | |
with no legend for a single response. Use False to suppress legend. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This spelling misprint occurs in several other locations of this PR, but I am only marking this one. (Easy to find with grep supress
.)
control/freqplot.py
Outdated
omega_num=omega_num).plot(**kwargs) | ||
*args, omega=None, omega_limits=None, omega_num=None, | ||
Hz=False, **kwargs): | ||
"""Plot the response of the "Gange of 4" transfer functions for a system. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"""Plot the response of the "Gange of 4" transfer functions for a system. | |
"""Plot the response of the "Gang of 4" transfer functions for a system. |
doc/plotting.rst
Outdated
labels, but no legend is generated in the plot (this can also be | ||
accomplished by setting ``legend_map`` to ``False``. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
labels, but no legend is generated in the plot (this can also be | |
accomplished by setting ``legend_map`` to ``False``. | |
labels, but no legend is generated in the plot. (This can also be | |
accomplished by setting ``legend_map`` to ``False``.) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or delete (
and make the comment a non-parenthetical sentence.
doc/plotting.rst
Outdated
the ``legend_loc`` keyword argument is set to a string or integer, it | ||
will set the position of the legend as described in the | ||
:func:`matplotlib.legend` documentation. Finally, ``legend_map`` can be | ||
set to an` array that matches the shape of the subplots, with each item |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
set to an` array that matches the shape of the subplots, with each item | |
set to an array that matches the shape of the subplots, with each item |
doc/plotting.rst
Outdated
|
||
The :func:`~control.bode_plot`, :func:`~control.time_response_plot`, | ||
and selected other commands can also accept a matplotlib format | ||
string (e.g., 'r--'). The format string must appear as a positional |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
string (e.g., 'r--'). The format string must appear as a positional | |
string (e.g., ``'r--'``). The format string must appear as a positional |
Without the code delimiters, Sphinx renders --
as an em dash.
control/freqplot.py
Outdated
If present, replace automatically generated label(s) with the given | ||
label(s). If sysdata is a list, strings should be specified for each | ||
system. | ||
label_freq : int, optiona | ||
Label every nth frequency on the plot. If not specified, no labels | ||
are generated. | ||
legend_loc : int or str, optional | ||
Include a legend in the given location. Default is 'center right', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Include a legend in the given location. Default is 'center right', | |
Include a legend in the given location. Default is 'upper right', |
control/pzmap.py
Outdated
label(s). If data is a list, strings should be specified for each | ||
system. | ||
legend_loc : int or str, optional | ||
Include a legend in the given location. Default is 'center right', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Include a legend in the given location. Default is 'center right', | |
Include a legend in the given location. Default is 'upper right', |
control/pzmap.py
Outdated
use matplotlib.pyplot.gca().axis('auto') and then set the axis | ||
limits to the desired values. | ||
|
||
2. Pole/zero plts that use the continuous time omega-damping grid do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2. Pole/zero plts that use the continuous time omega-damping grid do | |
2. Pole/zero plots that use the continuous time omega-damping grid do |
control/pzmap.py
Outdated
2. Pole/zero plts that use the continuous time omega-damping grid do | ||
not work with the ``ax`` keyword argument, due to the way that axes | ||
grids are implemented. The ``grid`` argument must be set to | ||
``False`` or `'empty'`` when using the ``ax`` keyword argument. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
``False`` or `'empty'`` when using the ``ax`` keyword argument. | |
``False`` or ``'empty'`` when using the ``ax`` keyword argument. |
control/freqplot.py
Outdated
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. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The return type here is ControlPlot
.
Thanks for the careful review, @slivingston. Will address all of these in the coming day or two and try to merge this weekend (two additional, fairly minor, PRs coming after that). |
Good catch! This was not intentional and there is now an inconsistency between tickmark labels in MIMO Bode plots versus MIMO time response plots. Since this is already a very large PR, I'm going to issue a separate PI to make that more uniform (and controllable). Remaining changes have been implemented. |
Something odd going on in the unit tests: |
3270392
to
9f143ec
Compare
The file originates in #1022 so may need to rebase or merge. |
The CI test can be fixed by adding a |
This error is not triggered on |
This PR makes a (fairly large) number of changes to control plotting functions to provide consistent functionality. The majority of changes involve making functionality that was present in some plot functions but not others available consistently across all _plot() functions. Everything is backward compatible with v0.10.0.
Summary of changes:
ControlPlot
object, with lines, axes, legend, etc available. Accessing this object as a list is backward compatible with 10.0 format (with deparecation warning).ax
keyword consistent across all plotting functions (usingctrlplot._process_ax_keyword
).label
keyword to operate in a consistent and more intuitive manner: labels can be specified as a single string, a simple list, or an array (for MIMO and multi-trace systems). If a single list is given for a MIMO or multi-trace system, it is reshaped as needed.ct.suptitle()
tocplt.set_plot_title()
(wherecplt
is the returnedControlPlot
obect from a plotting command), and update theplot()
method to provide uniform processing of the title keyword (when present, overrides title).relabel
keyword intime_response_plot
. This didn't seem to be that useful and was not implemented for other functions.rcParams
for control plotting functions: defaults are now inct.rcParams
and can be reset usingct.reset_rcParams
. Set up uniform processing of thercParams
keyword argument for plotting functions (with unit tests).color
and*fmt
argument processing code, in addition to color management for sequential plotting (_get_color_offset
,_get_color
).ctrlplot.py
.Documentation of new functionality (from
plotting.rst
):Customizing control plots
A set of common options are available to customize control plots in various ways. The following general rules apply:
If a plotting function is called multiple times with data that generate control plots with the same shape for the array of subplots, the new data will be overlaid with the old data, with a change in color(s) for the new data (chosen from the standard matplotlib color cycle). If not overridden, the plot title and legends will be updated to reflect all data shown on the plot.
If a plotting function is called and the shape for the array of subplots does not match the currently displayed plot, a new figure is created. Note that only the shape is checked, so if two different types of plotting commands that generate the same shape of subplots are called sequentially, the
matplotlib.pyplot.figure
command should be used to explicitly create a new figure.The
ax
keyword argument can be used to direct the plotting function to use a specific axes or array of axes. The value of theax
keyword must have the proper number of axes for the plot (so a plot generating a 2x2 array of subplots should be given a 2x2 array of axes for theax
keyword).The
color
,linestyle
,linewidth
, and other matplotlib line property arguments can be used to override the default line properties. If these arguments are absent, the default matplotlib line properties are used and the color cycles through the default matplotlib color cycle.The :func:
~control.bode_plot
, :func:~control.time_response_plot
, and selected other commands can also accept a matplotlib format string (e.g., 'r--'). The format string must appear as a positional argument right after the required data argument.Note that line property arguments are the same for all lines generated as part of a single plotting command call, including when multiple responses are passed as a list to the plotting command. For this reason it is often easiest to call multiple plot commands in sequence, with each command setting the line properties for that system/trace.
The
label
keyword argument can be used to override the line labels that are used in generating the title and legend. If more than one line is being plotted in a given call to a plot command, thelabel
argument value should be a list of labels, one for each line, in the order they will appear in the legend.For input/output plots (frequency and time responses), the labels that appear in the legend are of the form "<output name>, <input name>, <trace name>, <system name>". The trace name is used only for multi-trace time plots (for example, step responses for MIMO systems). Common information present in all traces is removed, so that the labels appearing in the legend represent the unique characteristics of each line.
For non-input/output plots (e.g., Nyquist plots, pole/zero plots, root locus plots), the default labels are the system name.
If
label
is set toFalse
, individual lines are still given labels, but no legend is generated in the plot (this can also be accomplished by settinglegend_map
toFalse
.Note: the
label
keyword argument is not implemented for describing function plots or phase plane plots, since these plots are primarily intended to be for a single system. Standardmatplotlib
commands can be used to customize these plots for displaying information for multiple systems.The
legend_loc
,legend_map
andshow_legend
keyword arguments can be used to customize the locations for legends. By default, a minimal number of legends are used such that lines can be uniquely identified and no legend is generated if there is only one line in the plot. Settingshow_legend
toFalse
will suppress the legend and setting it toTrue
will force the legend to be displayed even if there is only a single line in each axes. In addition, if the value of thelegend_loc
keyword argument is set to a string or integer, it will set the position of the legend as described in thematplotlib.legend
documentation. Finally,legend_map
can be set to an` array that matches the shape of the subplots, with each item being a string indicating the location of the legend for that axes (orNone
for no legend).The
rcParams
keyword argument can be used to override the default matplotlib style parameters used when creating a plot. The default parameters for all control plots are given by thect.rcParams
dictionary and have the following values:Only those values that should be changed from the default need to be specified in the
rcParams
keyword argument. To override the defaults for all control plots, update thect.rcParams
dictionary entries.The default values for style parameters for control plots can be restored using :func:
~control.reset_rcParams
.The
title
keyword can be used to override the automatic creation of the plot title. The default title is a string of the form " plot for " where is a list of the sys names contained in the plot (which can be updated if the plotting is called multiple times). Usetitle=False
to suppress the title completely. The title can also be updated using thecontrol.ControlPlot.set_plot_title
method for the returned control plot object.The plot title is only generated if
ax
isNone
.The following code illustrates the use of some of these customization features::