Switch LTI class and subclasses to use ninputs, noutputs, nstates #515
Conversation
|
Awesome! Looks good to me. |
|
One more thing I noticed: there are a few mentions of sys.inputs/outputs in docs in xferfunc.py that should probably be fixed. |
374c82f to
5a4c3ab
Compare
|
Rebased against master (and apparently fixed up the xferfcn.py docstring, since I can't find sys.inputs in that file anymore). |
|
May also need to check for self.inputs ?
|
d614c01 to
f633874
Compare
This matches the syntax we use in the |
|
That makes sense. Thanks! |
control/lti.py
Outdated
| raise PendingDeprecationWarning( | ||
| "The LTI `inputs` attribute will be deprecated in a future " | ||
| "release. Use `ninputs` instead.") | ||
| return self.ninputs |
There was a problem hiding this comment.
This raises an Exception of the class PendingDeprecationWarning and never returns self.inputs.
Python 3.8.6 (default, Nov 09 2020, 12:09:06) [GCC] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import control
>>> rsys = control.rss(3,3,3)
>>> ninputs = rsys.inputs
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File "/home/ben/src/python-control/control/lti.py", line 64, in inputs
raise PendingDeprecationWarning(
PendingDeprecationWarning: The LTI `inputs` attribute will be deprecated in a future release. Use `ninputs` instead.
>>> ninputs
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
NameError: name 'ninputs' is not defined
| raise PendingDeprecationWarning( | |
| "The LTI `inputs` attribute will be deprecated in a future " | |
| "release. Use `ninputs` instead.") | |
| return self.ninputs | |
| warn("The LTI `inputs` attribute will be deprecated in a future " | |
| "release. Use `ninputs` instead.", | |
| PendingDeprecationWarning, stacklevel=2) | |
| return self.ninputs |
Additionally, either a warning filter needs to be set in order to make the warning visible to end users, or a different category than *DeprecationWarning needs to be used.
filterwarnings('module',
message=".*LTI.*attribute.*deprecated",
category=PendingDeprecationWarning)Python 3.8.6 (default, Nov 09 2020, 12:09:06) [GCC] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import control; rsys = control.rss(3,3,3); ninputs = rsys.inputs
<stdin>:1: PendingDeprecationWarning: The LTI `inputs` attribute will be deprecated in a future release. Use `ninputs` instead.
>>> ninputs
3
- Please add a check using pytest.warns().
There was a problem hiding this comment.
I've fixed these up and decide to use DeprecationWarning for now (keep things internal to developers) but in a future release (0.9.x) we should change to FutureWarning, which will make it show up for users as well.
There was a problem hiding this comment.
Still using raise instead of warn. Now we are at:
This raises an Exception of the class
DeprecationWarningand never returns self.inputs.
Python 3.8.6 (default, Nov 09 2020, 12:09:06) [GCC] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import control; rsys = control.rss(3,3,3); ninputs = rsys.inputs
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File "/home/ben/src/python-control/control/lti.py", line 64, in inputs
raise DeprecationWarning(
DeprecationWarning: The LTI `inputs` attribute will be deprecated in a future release. Use `ninputs` instead.
>>> ninputs
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
NameError: name 'ninputs' is not defined
>>>
control/tests/lti_test.py
Outdated
| with pytest.raises(DeprecationWarning, match="LTI `inputs`"): | ||
| assert sys.inputs == sys.ninputs |
There was a problem hiding this comment.
The assert .. == part is never evaluated after sys.ninputs throws the exception.
There was a problem hiding this comment.
Sorry, was working on multiple changes/branches at once and pushed the wrong version. New one was what was intended.
bed8990 to
ff5fb39
Compare
ff5fb39 to
eb401a9
Compare
This PR changes the
LTIclass and theStateSpaceclass (and all derived classes) to useninputs,noutputs, andnstatesas the attributes for storing these properties instead ofinputs,outputs, andstates. The former names are consistent with what is used in theInputOutputSystemclass.Following the suggestion of @bnavigator in issue #451, getter and setter functions are used for backward compatibility, but with a
PendingDeprecationWarning.Note that this PR will generate some small conflicts with #514. I'll rebase based on which one we merge first.[done]