recalculate loci for sisotool and rlocus on axis scaling by repagh · Pull Request #1153 · python-control/python-control

repagh · 2025-06-05T19:37:41Z

adds recalculate of loci for pan/zoom for the root_locus_map and sisotool . Fixes #1151

coveralls · 2025-06-05T22:08:44Z

coverage: 94.759% (+0.01%) from 94.746%
when pulling b588045 on repagh:zoom-rlocus
into 632391c on python-control:main.

Copilot

Pull Request Overview

This PR enables dynamic recalculation of root locus and pole-zero plots when the user pans or zooms in sisotool and rlocus, fixing issue #1151.

Integrate add_loci_recalculate callback into sisotool and root_locus_plot for auto-replot on axis changes
Extend root_locus_map signature to accept xlim, ylim and pass them through to gain calculation
Add replot support for pole-zero maps via a new pole_zero_replot function

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File	Description
control/sisotool.py	Imported and invoked `add_loci_recalculate`, updated axis reference for root locus
control/rlocus.py	Added `xlim`/`ylim` parameters, defined `add_loci_recalculate`, wired callback in `root_locus_plot`
control/pzmap.py	Added `replot` method and `pole_zero_replot` for pole-zero redraw on zoom/pan

Comments suppressed due to low confidence (2)

control/sisotool.py:141

Variable axes is undefined here; you likely meant to use fig.axes or the local axes variable instead of axes.

ax_rlocus = axes[0,1] #fig.axes[1]

control/pzmap.py:552

ControlPlot instances expose their axes as cp.axes, not cp.ax. Update to cp.axes[0,0] to avoid AttributeError.

cp.lines[idx,2].append(cp.ax[0,0].plot(

Copilot · 2025-06-06T06:52:15Z

control/sisotool.py

+    if event.inaxes == ax.axes:
+
        fig = ax.figure



[nitpick] The original toolbar mode check for 'zoom rect' and 'pan/zoom' was removed, which may trigger click events during zooming or panning. Consider re-adding that guard to prevent unintended callbacks.

Suggested change

if event.inaxes == ax.axes:

fig = ax.figure

fig = ax.figure

toolbar = fig.canvas.toolbar

if toolbar is not None and toolbar.mode in ['zoom rect', 'pan/zoom']:

return

if event.inaxes == ax.axes:

In my experience, the risk of interpreting a click as a valid gain selection is small, and in my testing (with a print statement indicating when the callback is invoked), the zoom and pan events did not propagate to this callback. Removing the zoom_rect and pan/zoom check also removes unintended mode change (updating of the remaining 3 plots is inhibited when zoom / pan is selected), which can be confusing to users.

Copilot · 2025-06-06T06:52:15Z

control/rlocus.py

+    # Add a reaction to axis scale changes, if given LTI systems, and
+    # there is no set of pre-defined gains
+    if gains is None:
+        add_loci_recalculate(sysdata, cplt, cplt.axes[0,0])


[nitpick] Recalculating the entire locus on every xlim/ylim change can be expensive. Consider debouncing these callbacks or limiting the update frequency during continuous zooming.

This was no issue when interacting with the root-locus plot. Default number of points calculate is a measly 50, with the adaptive code in _default_gains typically a maximum between 100 and 200 points was produced. The callback will not be activated when the user manually specifies the gains.

murrayrm

I need to do a bit of testing, but here are some code style comments, mainly in docstrings (see online style guide, linked below).

murrayrm · 2025-06-13T17:56:20Z

control/pzmap.py

+        Parameters
+        ----------
+        cplt: ControlPlot
+            graphics handles of the existing plot


For consistency with python-control docstring standards, this should start with a capital letter and end in a period. See https://python-control.readthedocs.io/en/latest/develop.html#documentation-guidelines

amended commit

murrayrm · 2025-06-13T17:57:23Z

control/pzmap.py



+def pole_zero_replot(pzmap_responses, cp):
+    """Update the loci of a plot after zooming/panning


Should end in a period.

amended commit

murrayrm · 2025-06-13T17:58:38Z

control/pzmap.py

+    ----------
+    pzmap_responses : PoleZeroMap list
+        Responses to update
+    cp : ControlPlot


Is there a reason to use cp here instead of cplt, as above?

amended commit

murrayrm · 2025-06-13T18:00:32Z

control/rlocus.py



+def add_loci_recalculate(sysdata, cplt, axis):
+    """ Add a calback to re-calculate the loci data fitting a zoom action


Fix typo (callback) and end in period.

amended commit

murrayrm · 2025-06-13T18:03:59Z

control/rlocus.py

        Gains to use in computing plot of closed-loop poles.  If not given,
        gains are chosen to include the main features of the root locus map.
+    xlim : array_like, 2 elements, optional
+        Plot range


Add period. Perhaps also expand description (see other uses of xlim, for example in control.root_locus_plot.

amended commit

murrayrm · 2025-06-13T18:06:24Z

control/sisotool.py

    #     ax=fig.axes[1])
-    ax_rlocus = fig.axes[1]
-    root_locus_map(sys[0, 0]).plot(
+    ax_rlocus = axes[0,1] #fig.axes[1]


PEP8: two spaces before #, one space after.

amended commit

repagh force-pushed the zoom-rlocus branch from 7d567d6 to 66a3593 Compare June 5, 2025 21:52

repagh requested a review from Copilot June 6, 2025 06:50

Copilot AI reviewed Jun 6, 2025

View reviewed changes

murrayrm reviewed Jun 13, 2025

View reviewed changes

repagh force-pushed the zoom-rlocus branch from 66a3593 to 2877443 Compare June 18, 2025 07:12

recalculate loci for sisotool and rlocus on axis scaling

b588045

repagh force-pushed the zoom-rlocus branch from 2877443 to b588045 Compare June 19, 2025 11:11

murrayrm merged commit 34c6d59 into python-control:main Jun 21, 2025
24 checks passed

murrayrm added this to the 0.10.2 milestone Jul 4, 2025



		def pole_zero_replot(pzmap_responses, cp):
		"""Update the loci of a plot after zooming/panning



		def add_loci_recalculate(sysdata, cplt, axis):
		""" Add a calback to re-calculate the loci data fitting a zoom action

Conversation

repagh commented Jun 5, 2025

Uh oh!

coveralls commented Jun 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Jun 6, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jun 6, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

murrayrm left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Comments

coveralls commented Jun 5, 2025 •

edited

Loading