GeezerVison is showing github.com — Two 'sgrid' functions when we only need one? · Issue #634 · python-control/python-control
Two 'sgrid' functions when we only need one? #634DescriptionAs pointed out in PR #617, there are now two sgrid functions in the package: grid.py:sgrid and rlocus.py:_sgrid_func. It looks like _sgrid_func was removed in PR #193 (discrete omega-damping plot and tweaks by Sup3rGeo), but was re-introduced in PR #209 (sisotool and dynamic root locus zoom by icam0). Activitybnavigator commented on Jun 16, 2021 This should be fixed together with #632 and #623 billtubbs commented on Aug 15, 2021 I'm planning to look at this tomorrow. Does anyone know why rlocus.py calls its own local _sgrid_func(fig=None, zeta=None, wn=None) instead of grid.sgrid() (which is already imported into the module but is not used). There might be a reason why someone did this...
See these lines:
|
# Draw the grid |
|
if grid: |
|
if isdtime(sys, strict=True): |
|
zgrid(ax=ax) |
|
else: |
|
_sgrid_func(fig=fig if sisotool else None) |
bnavigator commented on Aug 15, 2021 billtubbs commented on Aug 15, 2021 Okay, thanks. So more of an accident than an intention. billtubbs commented on Aug 15, 2021 Okay, I've run into the same problem from last time I looked at this. The sgrid() function is not compatible with the plotting paradigms we wanted to employ for figures with multiple subplots:
plt.subplot(2, 1, 1)
zgrid() # this works
plt.subplot(2, 1, 2)
sgrid() # this won't
plt.show() and
fig, axes = plt.subplots(2, 1)
zgrid(ax=axes[0, 0]) # this works
sgrid(ax=axes[1, 0]) # this won't
plt.show() This won't work because sgrid uses a PolarAxes.PolarTransform to create the axis, which you have to do at the point of creating the axis, not after one already exists:
|
grid_helper = GridHelperCurveLinear( |
|
tr, extreme_finder=extreme_finder, grid_locator1=grid_locator1, |
|
tick_formatter1=tick_formatter1) |
|
|
|
fig = plt.gcf() |
|
ax = SubplotHost(fig, 1, 1, 1, grid_helper=grid_helper) |
Also, I don't think there's a way to remove an existing axis and then replace it with the desired polar axis (could be wrong on this point).
Welcome comments on this but I think there is one solution... billtubbs commented on Aug 15, 2021 The only solution I can think of right now is to require a slightly different set of arguments when adding an sgrid to an existing figure. Based on this example script from the documentation, I think this might work but I haven't tried implementing it yet:
fig = plt.figure()
sgrid(fig=fig, position=(2, 1, 1)) # this should be do-able
zgrid(fig=fig, position=(2, 1, 2)) # add the same functionality to zgrid
plt.show() From the documentation, it sounds like the subplots method was intended as a convenience method for making plots of the same type only. It does have two keyword arguments for creating different types of axes and grids, subplot_kw and gridspec_kw but I'm not sure you can specify different types for each subplot (I will check this). billtubbs commented on Aug 15, 2021 Question. I'm in the process of replacing _sgrid_func with sgrid and I noticed that _sgrid_func uses two other functions in rlocus.py:
|
def _default_zetas(xlim, ylim): |
|
"""Return default list of damping coefficients |
|
|
|
This function computes a list of damping coefficients based on the limits |
|
of the graph. A set of 4 damping coefficients are computed for the x-axis |
|
and a set of three damping coefficients are computed for the y-axis |
|
(corresponding to the normal 4:3 plot aspect ratio in `matplotlib`?). |
|
def _default_wn(xloc, yloc, max_lines=7): |
|
"""Return default wn for root locus plot |
|
|
|
This function computes a list of natural frequencies based on the grid |
|
parameters of the graph. |
Shall I delete these functions (they were only used by _sgrid_func) or should these features be carried over? sawyerbfuller commented on Aug 16, 2021 Hi bill, I’ve never seen them used anywhere else, and as private functions
I think you can delete them if they don’t show up in anywhere else in the
library.
--
Sawyer Fuller
Assistant Professor of Mechanical Engineering
Adjunct, Paul G Allen School of Computer Science
University of Washington
http://faculty.washington.edu/~minster/
(Typed with my thumbs) bnavigator commented on Aug 16, 2021
fig, axes = plt.subplots(2, 1)
zgrid(ax=axes[0, 0]) # this works
sgrid(ax=axes[1, 0]) # this won't
plt.show()
fig = plt.figure()
sgrid(fig=fig, position=(2, 1, 1)) # this should be do-able
zgrid(fig=fig, position=(2, 1, 2)) # add the same functionality to zgrid
plt.show()
You can always get the parent figure of a given axis by ax.get_figure() and the position by ax.get_subplotspec(). So whatever you can achieve by the second form should also be possible with the first form. bnavigator commented on Aug 16, 2021
This won't work because sgrid uses a PolarAxes.PolarTransform to create the axis, which you have to do at the point of creating the axis, not after one already exists:
|
grid_helper = GridHelperCurveLinear( |
|
tr, extreme_finder=extreme_finder, grid_locator1=grid_locator1, |
|
tick_formatter1=tick_formatter1) |
|
|
|
fig = plt.gcf() |
|
ax = SubplotHost(fig, 1, 1, 1, grid_helper=grid_helper) |
Also, I don't think there's a way to remove an existing axis and then replace it with the desired polar axis (could be wrong on this point).
The MPL Curvilinear grid demo where this was taken from has changed a little bit since then. Maybe this needs an update too. billtubbs commented on Aug 16, 2021
You can always get the parent figure of a given axis by ax.get_figure() and the position by ax.get_subplotspec(). So whatever you can achieve by the second form should also be possible with the first form.
The problem is not getting access to the figure, it is that the axes has to be created in a certain way by sgrid. sgrid cannot work with an axis that is already created so it has to be a blank figure (before the axis was created). Unless someone knows a way to remove an axis already created so it can be replaced. bnavigator commented on Aug 16, 2021 I am not sure about this. Maybe you can shuffle around existing axes objects together with a newly created parasite axis using the right combination of delaxes(ax), fig.add_subplot(ax), etc. billtubbs commented on Aug 16, 2021 Thanks, delaxes(ax) looks promising. I will check out later this week.
MetadataMetadataDevelopmentNo branches or pull requestsIssue actions
|