Skip to content

Comments

klist->gains kwarg for matlab.wrappers.rlocus#999

Merged
murrayrm merged 2 commits intopython-control:mainfrom
gdmcbain:998-matlab-rlocus-klist
May 11, 2024
Merged

klist->gains kwarg for matlab.wrappers.rlocus#999
murrayrm merged 2 commits intopython-control:mainfrom
gdmcbain:998-matlab-rlocus-klist

Conversation

@gdmcbain
Copy link
Contributor

@gdmcbain gdmcbain commented May 10, 2024

This fixes #998.

It wasn't the actual problem there, with control.matlab.wrappers.rlocus not being called (rather control.rlocus.rlocus = control.rlocus.root_locus_plot), but still this does forward kwargs to that function which is expecting gains rather than klist or kvect (though it does have a control._process_legacy_keyword in place to catch the latter).

Feel free to ignore or use this.

@slivingston slivingston self-assigned this May 10, 2024
@slivingston
Copy link
Member

slivingston commented May 10, 2024

Thanks, @gdmcbain ! While reviewing this, I noticed another kvect in the same docstring, so I fixed it, too.

By the way, it looks like the email address you are using in the commit is not associated with your GitHub account because your commit appears without an avatar above. Is that intentional, or did you want to use a different address?

@coveralls
Copy link

coveralls commented May 10, 2024

Coverage Status

coverage: 94.503%. remained the same
when pulling cb4244d on gdmcbain:998-matlab-rlocus-klist
into e4a03e8 on python-control:main.

@bnavigator
Copy link
Contributor

bnavigator commented May 11, 2024

How about mapping MATLAB's parameter k to the gains kwarg of root_locus_plot()? https://www.mathworks.com/help/control/ref/dynamicsystem.rlocus.html

@murrayrm murrayrm merged commit 2d1f514 into python-control:main May 11, 2024
@gdmcbain gdmcbain deleted the 998-matlab-rlocus-klist branch May 13, 2024 01:30
gdmcbain added a commit to gdmcbain/python-control that referenced this pull request May 13, 2024
@gdmcbain
process legacy keyword k->gains python-control#999
206ca85
@gdmcbain
Copy link
Contributor Author

gdmcbain commented May 13, 2024

By the way, it looks like the email address you are using in the commit is not associated with your GitHub account because your commit appears without an avatar above. Is that intentional, or did you want to use a different address?

Oops. I assume it's too late to fix now; never mind.

How about mapping MATLAB's parameter k to the gains kwarg of root_locus_plot()? https://www.mathworks.com/help/control/ref/dynamicsystem.rlocus.html

Sure, why not. Let's.

@gdmcbain gdmcbain mentioned this pull request May 13, 2024
Merged
@murrayrm murrayrm added this to the 0.10.1 milestone Jun 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@slivingston slivingston slivingston approved these changes

Assignees

@slivingston slivingston

Labels

None yet

Projects

None yet

Milestone

0.10.1

Development

Successfully merging this pull request may close these issues.

inconsistent kwarg klist/kvect/gains in rlocus doc

5 participants

@gdmcbain @slivingston @coveralls @bnavigator @murrayrm