Add slicing access for state-space models with tests#1012
Add slicing access for state-space models with tests#1012murrayrm merged 4 commits intopython-control:mainfrom
Conversation
|
@ guptavaibhav0 Please rebase this on the last main branch to allow the CI tests to run. Please note that the failure of "Conda-based pytest / Py3.10; conda Slycot; no Pandas; no CVXOPT" looks like something unrelated and should clear up when the tests are re-run. |
|
@murrayrm I have done the rebase and hopefully, this should fix it! |
control/xferfcn.py
Outdated
|
|
||
| def __getitem__(self, key): | ||
| if not isinstance(key, Iterable) or len(key) != 2: | ||
| raise IOError('must provide indices of length 2 for state space') |
There was a problem hiding this comment.
Change "state space" to "transfer functions".
There was a problem hiding this comment.
I have updated in the latest commit.
| [(0, 1), | ||
| (slice(0, 1, 1), 1), | ||
| (0, slice(1, 2, 1)), | ||
| (slice(0, 1, 1), slice(1, 2, 1))]) |
There was a problem hiding this comment.
It would be nice to add some more general tests. For example, how about [:2, 1] or [::-1, ::2]?
There was a problem hiding this comment.
I have added some more general tests in the new commit. I also found a bug in the StateSpace init function which I have corrected. The init function doesn't check if the dimensions of the D matrix are correct if the input/output names are given.
control/statesp.py
Outdated
| self.dt, name=sysname, | ||
| inputs=[self.input_labels[i] for i in list(inpdx)], | ||
| outputs=[self.output_labels[i] for i in list(outdx)]) | ||
| self.dt, name=sysname, inputs=self.input_labels[inpdx], outputs=self.output_labels[outdx]) |
There was a problem hiding this comment.
Was there a reason for this change? It looks like the updated final line goes past column 79, which we try not to do.
There was a problem hiding this comment.
The change was needed since slice is not iterable. I have pushed a new commit with a maximum column length of 79.
This PR adds the capability to access state-space models using splice. This should help fix #256.
Current Issue:
Changes in this PR: