Add unit test illustrating issue #935 + add method keyword for tf2ss #937
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
sawyerbfuller
approved these changes
Oct 26, 2023
| @@ -2250,6 +2274,8 @@ def _convert_to_statespace(sys, use_prefix_suffix=False): | |||
| A, B, C, D = \ | |||
There was a problem hiding this comment.
Suggested change for the two lines above:
if not issiso(sys):
raise ControlMIMONotImplemented("MIMO system conversion not supported without Slycot")
| @@ -2250,6 +2274,8 @@ def _convert_to_statespace(sys, use_prefix_suffix=False): | |||
| A, B, C, D = \ | |||
| sp.signal.tf2ss(squeeze(sys.num), squeeze(sys.den)) | |||
| newsys = StateSpace(A, B, C, D, sys.dt) | |||
| else: | |||
| raise ValueError(f"unknown {method=}") | |||
There was a problem hiding this comment.
First instantiation of self-documenting F-strings in python-control? : )
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This PR puts in some unit tests associated with issue #935, which looks like it is a problem in slycot where the tf2ss functionality (in
td04ad) doesn't work properly. As a partial workaround, amethodkeyword is added totf2ss(andss) that allowsmethod='scipy'to be used for SISO systems. Docstrings updated for the new keyword and a note on theslycotissue.A bug report has been created in the
slycotrepository (#222 and the unit tests will fail when the bug is fixed (so we can go back and update the unit tests and documentation).