Skip to content

Fix root_locus() handling of ax parameter #871

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.

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

Merged
merged 1 commit into from
Feb 23, 2023
Merged

Fix root_locus() handling of ax parameter #871

merged 1 commit into from
Feb 23, 2023

Conversation

henklaak
Copy link
Contributor

Fixes #870

@coveralls
Copy link

Coverage Status

Coverage: 94.883% (+0.01%) from 94.872% when pulling d13f1ed on henklaak:fix_root_locus_ax into 346bc40 on python-control:main.

@sawyerbfuller
Copy link
Contributor

This seems like a more straightforward and correct way of implementing sgrid - can you confirm sisotool still works as you might expect on that transfer function, and that the gridlines turn off when rlocus_grid=False?

Also, we currently have an excess of sgrid functions. There is another : ) It is to be found in grid.py and it is called sgrid. I haven't compared the two but that one may be more modern/faster? (though it also needs to be upgraded to accept an ax keyword). If so, maybe the root_locus function should switch to using that one and delete the one in rlocus.

@henklaak
Copy link
Contributor Author

@sawyerbfuller
Sisotool still works (as far as I know how to use it) and the grid is 'switchable'.

The harmonization of different grid implementations is outside the scope of this Issue/PR. I can take a stab at it later.

image

@sawyerbfuller
Copy link
Contributor

Looks good to me

@sawyerbfuller sawyerbfuller merged commit 8972e82 into python-control:main Feb 23, 2023
@murrayrm murrayrm added this to the 0.9.4 milestone Mar 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

root_locus() does not properly handle ax parameter
4 participants