- Notifications
You must be signed in to change notification settings - Fork441
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
to480cda7
Comparecontrol/lti.py Outdated
@@ -395,8 +389,7 @@ def evalfr(sys, x, squeeze=None): | |||
See Also | |||
-------- | |||
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__
forfrd
systems? )
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
raise NotImplementedError("dcgain not implemented for %s objects" % | ||
str(self.__class__)) | ||
"""Return the zero-frequency (DC) gain.""" | ||
return NotImplemented |
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?
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
to68835e4
CompareGreat 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) |
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:
|
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. |
“ State space models can be manipulated …” appears twice in part 3 |
In part 3,
|
Add |
Confirmed that |
I'm updating |
1fd18b4
to258e28e
CompareA few more things as I find them, concentrating on docs:
|
9dc0480
to7270f63
Compare@@ -1079,14 +1046,14 @@ def ctrb(A, B, t=None): | |||
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(), | ||
) | ||
except AssertionError: | ||
if platform.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
to7da8c00
Compare11d753f
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 doctest
command in thedoc/
directory (called automatically whenmake html
is run).custom.css
file to control HTML formatting:py:obj
directive. 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.py
andtemplate.rst
that implement the guidelines.numpydoc
checks incontrol/tests/docstring_test.py
to 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/examples
and documentation figures todoc/figures
to declutter thedoc
directory (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
.object
matlab/__init__
and replaced withdoc/matlab.rst
(part of Reference Manual)isort -m2
on all files to sort imports.control/tests/docstring_test.py
unit test checks:Small code changes:
acker
toplace_acker
(to be consistent withplace_varga
) and setacker = place_acker
.NamedSignal
class now has a__repr__
method that evaluates back to aNamedSignal
(similar to theInputOutputSystem
__repr__
method).Additional changes that are coming: