Add aliases of selected functions as member functions to LTI #1092
Conversation
There was a problem hiding this comment.
Thanks for these contributions. These look OK to me.
I think we need to put in some sort of unit test just to make sure the functions work as intended. This is perhaps as simple as just creating a state space system (which has all of the member functions defined), making sure all of the member functions point to the existing function, and then calling it with no argument and with some simple argument(s) to make sure we get a ControlPlot back.
control/lti.py
Outdated
| # conversions | ||
| to_ss: Callable | ||
| to_tf: Callable | ||
|
|
||
| # system interconnections | ||
| feedback: Callable | ||
|
|
||
| # freq domain plotting | ||
| bode_plot: Callable | ||
| nyquist_plot: Callable | ||
| nichols_plot: Callable | ||
|
|
||
| # time domain simulation | ||
| forced_response = control.timeresp.forced_response | ||
| impulse_response = control.timeresp.impulse_response | ||
| step_response = control.timeresp.step_response |
There was a problem hiding this comment.
We should add (short) docstrings for these. Since these are initialized as assigned member variables instead of standard function (using def), we will have to use the doc-comment format (lines starting with #: before the variable). See InputOutputSystem.nstates for an example.
I think these can be short, perhaps of the form: "Bode plot; see bode_plot function for more information".
control/tests/kwargs_test.py
Outdated
| @@ -244,6 +244,7 @@ def test_response_plot_kwargs(data_fcn, plot_fcn, mimo): | |||
| 'append': test_unrecognized_kwargs, | |||
| 'bode': test_response_plot_kwargs, | |||
| 'bode_plot': test_response_plot_kwargs, | |||
| 'LTI.bode_plot': test_response_plot_kwargs, | |||
There was a problem hiding this comment.
I would add a comment at the end of this line (and the equivalent lines below) indicating that this is tested via bode_plot since there is not a separate test for the member function (and it really isn't needed, since it is the same object).
…as for feedback and added stub to LTI
|
Hi, |
And add automatic frequency computation to
LTI.frequency_responsesince refactoring to replace it withcontrol.frequency_responseis not possible without breaking changes.Closes #1084