- Notifications
You must be signed in to change notification settings - Fork446
Updated documentation#1094
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
44e40f6 to480cda7Comparecontrol/lti.py Outdated
| -------- | ||
| freqresp | ||
| bode | ||
| frequency_response, bode_plot |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Possible to includeStatespace.__call__andTransferFunction.__call__ here, and also inevalfr below? (Is there also a__call__ forfrdsystems? )
also - my opinion is that evalfr should be deprecated and only left in the Matlab module. Or given a more pythonic name that serves as a wrapper for call. But in any case, evalfr appears nowhere else in the documentation and should not be referenced infrequency_response. but__call__ should be.
| ss | ||
| ss2tf | ||
| tf2ss | ||
| TransferFunction, ss, ss2tf, tf2ss |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
nlsys
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I looked into this, and I don't think we need to referencenlsys here. It is another factory function, but not that directly relevant to transfer functions. I will includenlsys in thess factory function docstring.
| tf | ||
| ss | ||
| tf2ss | ||
| tf, ss, tf2ss |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
nlsys
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
As above, will leavenlsys off here.
control/lti.py Outdated
| raiseNotImplementedError("dcgain not implemented for %s objects"% | ||
| str(self.__class__)) | ||
| """Return the zero-frequency (DC) gain.""" | ||
| returnNotImplemented |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
According tohttps://docs.python.org/3.12/library/exceptions.html#NotImplementedError , abstract methods should raise an error if they’re not overridden.
| phase_crossover_frequencies | ||
| singular_values_response | ||
| tfdata | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Margin
doc/xferfcn.rst Outdated
| margins as well as the frequencies at which they occur:: | ||
| >>> sys = ct.tf(10, [1, 2, 3, 4]) | ||
| >>> gm, pm, sm, wpc, wgc, wms = ct.stability(sys) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Stability_margins?
murrayrm commentedJan 18, 2025
Thanks for the comments@sawyerbfuller. I'll update these in the next push. I'm not sure what's up with the failing tests. It looks like there is a (non-transient) error in Netscape Portable Runtime (NSPR). |
d601d38 to68835e4Comparesawyerbfuller commentedJan 21, 2025
Great for newcomers that you’re working on this docs cleanup, I am in favor of the changes you’re working on. I’m not an expert in colab but I have often just run it with a is necessary in the intro sections? Apologies for bad formatting, I’m typing this on my phone) |
sawyerbfuller commentedJan 21, 2025
I’m not sure whether the introductory tutorial is new or not but it is certainly more discoverable now. Nicely showcases the new time and frequency response plotting facilities. A few notes:
|
murrayrm commentedJan 21, 2025
I think you are right that Colab won't re-download the package, but the try/except may still be needed if you want the notebook to work in Jupyter. I'll go back and check and update in the next code push if it isn't needed. |
sawyerbfuller commentedJan 21, 2025
“ State space models can be manipulated …” appears twice in part 3 |
sawyerbfuller commentedJan 21, 2025
In part 3,
|
sawyerbfuller commentedJan 21, 2025
Add |
murrayrm commentedJan 25, 2025
Confirmed that |
murrayrm commentedJan 25, 2025
I'm updating |
1fd18b4 to258e28eComparesawyerbfuller commentedJan 26, 2025
A few more things as I find them, concentrating on docs:
|
9dc0480 to7270f63Compare| defctrb(A,B,t=None): | ||
| """Controllabilty matrix. | ||
| Parameters |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
"Controllabilty" at the top of this docstring is now fixed onmain branch via#1107
control/tests/statesp_test.py Outdated
| try: | ||
| result= (sys**-1*sys).minreal() | ||
| expected=StateSpace([], [], [],np.eye(2),dt=0) | ||
| assert_tf_close_coeff( | ||
| ss2tf(expected).minreal(), | ||
| ss2tf(result).minreal(), | ||
| ) | ||
| exceptAssertionError: | ||
| ifplatform.system()=='Darwin': | ||
| pytest.xfail("minreal bug on MacOS") | ||
| else: | ||
| raise |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I'll plan to merge#1109 first, then rebase and remove this exception.
8f3701d to7da8c00Compare11d753f intopython-control:mainUh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
This (very large) PR contains a restructuring of the Sphinx documentation, including changes to improve uniformity of docstrings along with draft guidelines for developers. This is being posted initially as a draft PR so that people can start to have a look at the changes and provide feedback.
High level principles:
Because of the large number of files changed, this PR will probably be very difficult to review based on looking at the changes to individual files. It will probably make more sense to look through theupdated version of the documentation on ReadTheDocs.
Summary of significant changes:
make doctestcommand in thedoc/directory (called automatically whenmake htmlis run).custom.cssfile to control HTML formatting:py:objdirective. For classes, functions, and methods, this will generate a link using bold, code font. For parameters, non-bold text is used (since Sphinx does not yet support links to parameter documentation).ct.config.defaults(and a unit test to make sure nothing is missing).template.pyandtemplate.rstthat implement the guidelines.numpydocchecks incontrol/tests/docstring_test.pyto pick up things like improperly labeled sections (e.g., "See also" instead of "See Also") and other errors.__init__docstrings (which is not included in the Sphinx documentation) to class documentation (with details in the factory function, when appropriate).doc/examplesand documentation figures todoc/figuresto declutter thedocdirectory (this accounts for many of the 226 files that have changed).doc/releases.Smaller changes:
doc/test_sphinx) to make sure all primary functions are documented in the Reference Manual.examples/template.py.objectmatlab/__init__and replaced withdoc/matlab.rst(part of Reference Manual)isort -m2on all files to sort imports.control/tests/docstring_test.pyunit test checks:Small code changes:
ackertoplace_acker(to be consistent withplace_varga) and setacker = place_acker.NamedSignalclass now has a__repr__method that evaluates back to aNamedSignal(similar to theInputOutputSystem__repr__method).Additional changes that are coming: