Repository navigation
DOC: fix docstring parameters that the functions do not accept - #1274
dylanpulver wants to merge 2 commits into
Conversation
slivingston
left a comment
slivingston
left a comment
There was a problem hiding this comment.
- There is something wrong with the commit authorship. Can you verify that the email address in it is verified for your GitHub account?
- Please disclose use of tools consistent with https://numpy.org/doc/stable/dev/ai_policy.html.
|
Besides my requests, these changes are good to merge. For posterity, Regarding |
ControlPlot.set_plot_title documented `fig`, but the method takes (self, title, frame) and uses self.figure, so there is no way to pass it. The Parameters block came across from suptitle(), which is deprecated in favour of this method and does take fig. phase_plot documented `func`; the parameter was renamed to `odefun`, so the keyword form raised TypeError.
Per review: the passthrough was never dropped, it is simply not reachable through ControlPlot.set_plot_title. _update_plot_title is called from outside this module with arguments that land in **kwargs, so that is where the description belongs. It had no docstring, so the entries for its other parameters come from the module: fig from `if fig is None: fig = plt.gcf()`, frame from the wording already used in set_plot_title, and use_existing from the title-merging branch below.
d31ffaf to
ec4fd52
Compare
|
@dylanpulver Thanks. Please disclose use of tools consistent with https://numpy.org/doc/stable/dev/ai_policy.html. Since you are opening a very large number of PRs in other, seemingly unrelated repositories, I assume you are using some (or all) automation. |
|
coverage: 94.757% (+0.01%) from 94.747% — dylanpulver:doc/phantom-parameter-names into python-control:main |
Two Parameters entries name an argument the function does not take.
ControlPlot.set_plot_titledocumentsfig, but the method is(self, title, frame='axes')and usesself.figure, so there is no way to pass it. The whole Parameters block came across fromsuptitle(), which is deprecated in favour of this method and does takefig.phase_plotdocumentsfunc; the parameter was renamed toodefun, sophase_plot(func=...)raises TypeError. The usage lines above it still readphase_plot(func, ...), which works because that form is positional.One thing I did not change, because it is your call rather than mine. The same block documents
**kwargs : matplotlib.pyplot.suptitle keywords, optional, andset_plot_titleneither accepts nor forwards them: the body calls_update_plot_title(title, fig=self.figure, frame=frame, use_existing=False). So deprecatingsuptitledropped that passthrough. Removing the entry and restoring the passthrough are both defensible, and the second is a behaviour change, so I left it alone. Happy to follow up with whichever you prefer.Docstrings only.
ruff check control/ctrlplot.py control/phaseplot.pypasses.