Description

jolenscki

opened on May 22, 2021

Issue body actions

I was trying today to plot the root locus of a transfer function, but wanted to pass a specific axes object to it, so I tried to use the ax parameter of the rlocus function:

from control import *
import matplotlib.pyplot as plt

s = tf([1, 0], 1)
G = 1/(s*(s+1))

fig, ax = plt.subplots(dpi=200)
rlocus(G, ax=ax)

But when doing this, the following error was raised:

---------------------------------------------------------------------------
UnboundLocalError                         Traceback (most recent call last)
<ipython-input-1-80498c4b1a54> in <module>
      6 
      7 fig, ax = plt.subplots(dpi=200)
----> 8 rlocus(G, ax=ax)

~\anaconda3\lib\site-packages\control\rlocus.py in root_locus(sys, kvect, xlim, ylim, plotstr, plot, print_gain, grid, ax, **kwargs)
    173 
    174         if print_gain and not sisotool:
--> 175             fig.canvas.mpl_connect(
    176                 'button_release_event',
    177                 partial(_RLClickDispatcher, sys=sys, fig=fig,

UnboundLocalError: local variable 'fig' referenced before assignment

Control version: 0.9.0

Activity

1. sawyerbfuller commented on May 22, 2021

   

   sawyerbfuller

   on May 22, 2021

   Contributor

   More actions

   Looks like you caught a bug, thanks! I think when we implemented this we had what you intended in mind but didn’t get around to testing it.

   Reacted by jolenscki
2. murrayrm

   added

   bug

   on Jun 5, 2021
3. billtubbs commented on Aug 9, 2021

   

   billtubbs

   on Aug 9, 2021

   Contributor

   More actions

   It's not too difficult to fix. I think we just need to unindent lines 171 and 172 by one tab:

   python-control/control/rlocus.py

   Lines 169 to 172 in 283f5e7

   	if ax is None:
   	ax = plt.gca()
   	fig = ax.figure
   	ax.set_title('Root Locus')
4. billtubbs commented on Aug 9, 2021

   

   billtubbs

   on Aug 9, 2021

   Contributor

   More actions

   I'm happy to implement this immediate fix (and #632 at the same time) and maybe add some unit tests, including for other plot functions where applicable. If I recall correctly, I was trying to do this a while back and the problem is that some plot funcs need to generate their own axis (e.g. radial plots) and some generate a figure containing more than one axis.

   Reacted by Ben Greiner
5. bnavigator commented on Aug 9, 2021

   

   bnavigator

   on Aug 9, 2021

   Contributor

   More actions

   If you manage to also fix #634 while you are at it, it would be a blast. Not sure if it is worth to be done before/without implementing the new plotting paradigm, proposed by you in #65 (comment)
6. billtubbs commented on Aug 9, 2021

   

   billtubbs

   on Aug 9, 2021

   Contributor

   More actions

   I'll have a look at #634. But I'm waiting for further input on the new plotting paradigm. That will require more thinking and discussion and probably take a while to figure out a way forward. There might be better ways to do it. I'm happy to try fixing the immediate bugs on the existing functions when I next get the chance.

   Reacted by Ben Greiner
7. sawyerbfuller

   linked a pull request that will close this issuenew Pid design function built on sisotool #662on Nov 4, 2021
8. sawyerbfuller

   added 3 commits that reference this issue on Nov 4, 2021

   fix for github python-control#623

   1eb9997

   fix for github python-control#623

   b2478f2

   fix for github python-control#623

   3dca645
9. bnavigator

   closed this as completedin #662on Dec 26, 2021

Issue actions