Skip to content

inconsistent kwarg klist/kvect/gains in rlocus doc #998

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
gdmcbain opened this issue May 9, 2024 · 3 comments · Fixed by #999
Closed

inconsistent kwarg klist/kvect/gains in rlocus doc #998

gdmcbain opened this issue May 9, 2024 · 3 comments · Fixed by #999

Comments

@gdmcbain
Copy link
Contributor

gdmcbain commented May 9, 2024

The first line of the docstring for rlocus suggests that there's a kwarg klist,

"""rlocus(sys[, klist, xlim, ylim, ...])

python-control/control/matlab/wrappers.py

Line 200 in e4a03e8

"""rlocus(sys[, klist, xlim, ylim, ...])

but in the list of parameters it's kvect.

python-control/control/matlab/wrappers.py

Line 212 in e4a03e8

kvect : array_like, optional

They're all just passed on to root_locus_plot anyway

python-control/control/matlab/wrappers.py

Line 247 in e4a03e8

retval = root_locus_plot(*args, **kwargs)

and it's expecting kvect, or, really, gains

python-control/control/rlocus.py

Line 163 in e4a03e8

gains = config._process_legacy_keyword(kwargs, 'kvect', 'gains', gains)

@gdmcbain
Copy link
Contributor Author

gdmcbain commented May 9, 2024

This is miscalled in pymoca/pymoca test/notebooks/Spring,ipynb:

    "control.rlocus(ss[0,0], klist=pl.logspace(-2,1,1000));\n",

@gdmcbain gdmcbain mentioned this issue May 9, 2024
Closed
@bnavigator
Copy link
Contributor

The first line of the docstring for rlocus suggests that there's a kwarg klist,

Only for the MATLAB wrapper though. The MATLAB docs call the parameter k by the way.
Using a MATLAB wrapper with a python kwarg is somehow unusual.

PR welcome.

This is miscalled in pymoca/pymoca test/notebooks/Spring,ipynb:

The MATLAB wrapper is not part of that call. The kwarg for control.rlocus has been kvect for 13 years: 8ebb73b.

gdmcbain pushed a commit to gdmcbain/python-control that referenced this issue May 10, 2024
rlocus(kvect) python-control#998
9262b1a
gdmcbain pushed a commit to gdmcbain/python-control that referenced this issue May 10, 2024
klist->gains python-control#998
c5e2639
@gdmcbain
Copy link
Contributor Author

Ah, yes, sorry I see now that control.rlocus doesn't call control.matlab.wrappers.rlocus at all but rather control.root_locus_plot; I had overlooked this:

python-control/control/rlocus.py

Line 452 in e4a03e8

rlocus = root_locus_plot

My real problem was pymoca/pymoca#324 so I'll look over there at fixing that. I'm not presently concerned with control.matlab, but I'll turn in the PR as requested, while I'm at it.

@gdmcbain gdmcbain mentioned this issue May 10, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

klist->gains kwarg for matlab.wrappers.rlocus gdmcbain/python-control
2 participants
@gdmcbain @bnavigator