GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — DOC: fix docstring parameters that the functions do not accept by dylanpulver · Pull Request #1274 · python-control/python-control

Conversation

@dylanpulver

@dylanpulver dylanpulver commented Nov 12, 2026

Copy link
Copy Markdown

Two Parameters entries name an argument the function does not take.

  • ControlPlot.set_plot_title documents fig, but the method is (self, title, frame='axes') and uses self.figure, so there is no way to pass it. The whole Parameters block came across from suptitle(), which is deprecated in favour of this method and does take fig.
  • phase_plot documents func; the parameter was renamed to odefun, so phase_plot(func=...) raises TypeError. The usage lines above it still read phase_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, and set_plot_title neither accepts nor forwards them: the body calls _update_plot_title(title, fig=self.figure, frame=frame, use_existing=False). So deprecating suptitle dropped 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.py passes.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
slivingston
slivingston requested changes Nov 12, 2026
View reviewed changes

@slivingston slivingston left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • 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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@slivingston

slivingston commented Nov 12, 2026

Copy link
Copy Markdown
Member

Besides my requests, these changes are good to merge. For posterity,

  • use of "func" originates in commit 53dfeb0
  • fig parameter was removed in commit 89cda3d

Regarding **kwargs, _update_plot_title() is used by code outside of this module (ctrlplot.py), and there are calls with arguments that appear in **kwargs, so the passthrough was not "dropped." It simply is not available through set_plot_title(). Therefore, I think the most useful docstring change would be to move the **kwargs description from set_plot_title() to _update_plot_title().

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

dylanpulver added 2 commits November 12, 2026 19:21
@dylanpulver
DOC: fix docstring parameters that the functions do not accept
9bad931
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.
@dylanpulver
DOC: move the **kwargs description to _update_plot_title
ec4fd52
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.
@dylanpulver
dylanpulver force-pushed the doc/phantom-parameter-names branch from d31ffaf to ec4fd52 Compare November 13, 2026 00:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@slivingston slivingston slivingston requested changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

2 participants

@dylanpulver @slivingston
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402520 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.