make it so rlocus does not always create a new figure, so it is like matlab and control.sisotool #413
Conversation
…ne, similar to matlab behavior. new argument in rlocus to specify figure to plot in
| @@ -71,7 +71,8 @@ | |||
|
|
|||
| # Main function: compute a root locus diagram | |||
| def root_locus(sys, kvect=None, xlim=None, ylim=None, | |||
| plotstr=None, plot=True, print_gain=None, grid=None, **kwargs): | |||
| plotstr=None, plot=True, print_gain=None, grid=None, fig=None, | |||
| **kwargs): | |||
There was a problem hiding this comment.
If you are going to add this option I think it is much better to pass in the matplotlib axes to plot on, not the figure. If you look at other libraries, that is the most common approach and offers the best way to compose multiple plot calls onto a given axes or set of axes.
There was a problem hiding this comment.
Makes a lot of sense to me. I did it this way partly because the existing software does a few extra things, like renaming the figure "root locus". And it instead names it "root locus 1", "root locus 2" etc if there are pre-existing "root locus" plots. I was hesitant to break that functionality.
My natural inclination would be to take that part out because it is complex, inflexible, and not in keeping with the conventions elsewhere in the library and, like you say, on plotting routines in general (e.g. pzmap re-uses current axes), but I didn't want to step on any toes.
There was a problem hiding this comment.
It may be ok to remove that figure name. It isn't a strong backwards compatibility break.
But you can access the figure an axis attached to with:
In [7]: import matplotlib.pyplot as plt
In [8]: fig, ax = plt.subplots()
In [10]: ax.figure
Out[10]: <Figure size 640x480 with 1 Axes>
In [11]: ax.figure == fig
Out[11]: True
So you can append or replace the title in the figure. This should probably have an option for it too though, i.e. modify_figure_name=True.
There was a problem hiding this comment.
Thanks Jason, that was super helpful, I used it to make some changes.
- Updated the PR to make it plot in the current axis instead of the current figure.
- removed deprecated pylab import in favor of matplotlib
- And OK, I took out the figure (re-)naming code. Instead it now just adds a title to the plot axes.
- Incidentally, this seems like it may have fixed the slow zooming problem referenced on lines 180-181. (at least it did for me)
…e, with an option to specify a desired matplotlib axis instead
|
I'm good with this change, but one of the lead devs will have to comment and approve. |
|
Everywhere else in |
rlocus did not have a way to do anything other than always create a new figure.