Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

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

Merged
murrayrm merged 93 commits intopython-control:mainfrommurrayrm:userguide-22Dec2024
Feb 3, 2025

Conversation

murrayrm
Copy link
Member

@murrayrmmurrayrm commentedJan 13, 2025
edited
Loading

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:

  • No documentation has been removed, only shifted around.
  • With a few small exceptions, to fix up inconsistencies, there are no changes to the code base and all previous code will run without change.

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:

  • Sphinx documentation is now divided into a User Guide, which provides a narrative style description of python-control package, and a Reference Manual, which has a listing of all functions, classes, package configuraton parameters, and other detailed information about the package.
  • Control plots included in the documentation are generated by the code in the documentation, using the Sphinx doctest extension. Figures are created using themake doctest command in thedoc/ directory (called automatically whenmake html is run).
  • Regularized the way that classes, functions, methods, parameters, built-ins, and code samples are annotated and created acustom.css file to control HTML formatting:
    • Classes, functions, methods, and parameters use single backticks, with no additional directives, and are treated using thepy: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).
    • Code samples use double backticks and are rendered in non-bold, black, fixed width font (versus the default red color).
    • Python built-ins (True, False, None) are written with no backticks. This was previously very non-uniform, and this style fits what is commonly used in the NumPy documentation.
  • Moved theControl Systems Classes chapter to the Reference Manual and added all classes in the package (previous it was just the I/O system classes).
  • Added a new chapterPackage Configuration Parameters to the Reference Manual, with documentation on everything available viact.config.defaults (and a unit test to make sure nothing is missing).
  • Added a new chapterDeveloper Notes to the reference manual that describes the various choices that were made in making the package structure and documentation uniform. Sections in the chapter:
    • Package Structure: how the package is organized (directories, files)
    • Naming Conventions: how to name files, classes, functions, etc
    • Documentation Guidelines: how to document various types of objects, including consistent use of backticks (with rationale), as well a description of what goes in the User Guide vs Reference Manual chapters.
    • Utility Functions: a relatively incomplete listing of some of the more common utility functions for developers.
    • Sample Files:template.py andtemplate.rst that implement the guidelines.
  • Got rid of docstring hashes for functions with variable arguments and instead used docstring signatures as a replacement for checking that parameters are documented.
  • Addednumpydoc 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.
  • Moved documentation from__init__ docstrings (which is not included in the Sphinx documentation) to class documentation (with details in the factory function, when appropriate).
  • Moved documentation examples files todoc/examples and documentation figures todoc/figures to declutter thedoc directory (this accounts for many of the 226 files that have changed).
  • [26 Jan 2025] Added Release Notes providing a (user-oriented) summary of changes in recent releases (with a summary of earlier releases). Release notes are contained indoc/releases.

Smaller changes:

  • Added Sphinx unit test (doc/test_sphinx) to make sure all primary functions are documented in the Reference Manual.
  • Updated file header information to be a standard form, shown inexamples/template.py.
  • Removed top-level class dependence onobject
  • Removed (unused) table inmatlab/__init__ and replaced withdoc/matlab.rst (part of Reference Manual)
  • Ranisort -m2 on all files to sort imports.
  • Improved consistency checking incontrol/tests/docstring_test.py unit test checks:
    • All function and parameter summaries now start with a capital letter and end with a period.
    • Added checking docstring format of "Returns" section, in addition to "Parameters".
  • Remove excess spaces throughout the package (since all files were touched).
  • Equations are now centered in Sphinx, instead of left justified.

Small code changes:

  • Renamedacker toplace_acker (to be consistent withplace_varga) and setacker = place_acker.
  • Identified source code lines that were more than 79 characters and reformatted (PEP 8).
  • [25 Jan 2025] TheNamedSignal class now has a__repr__ method that evaluates back to aNamedSignal (similar to theInputOutputSystem__repr__ method).

Additional changes that are coming:

  • Regularize frequency response arguments/return vals: (fresp, freqpts)
  • Regularize timebase documentation ('dt' format + wording)
  • Make sure all MATLAB functions are documented in Reference Manual
  • Get rid of numbered notes (just use paragraphs)
  • Remove all remaining .. todo::'s
  • Fix additional typos and grammatical inconsistencies

@murrayrmmurrayrm marked this pull request as draftJanuary 13, 2025 06:11
@murrayrmmurrayrm marked this pull request as ready for reviewJanuary 15, 2025 00:13
@murrayrmmurrayrm marked this pull request as draftJanuary 15, 2025 00:14
@@ -395,8 +389,7 @@ def evalfr(sys, x, squeeze=None):

See Also
--------
freqresp
bode
frequency_response, bode_plot
Copy link
Contributor

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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

nlsys

Copy link
MemberAuthor

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.

sawyerbfuller reacted with thumbs up emoji
tf
ss
tf2ss
tf, ss, tf2ss
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

nlsys

Copy link
MemberAuthor

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.

sawyerbfuller reacted with thumbs up emoji
raise NotImplementedError("dcgain not implemented for %s objects" %
str(self.__class__))
"""Return the zero-frequency (DC) gain."""
return NotImplemented
Copy link
Contributor

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

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Margin

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)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Stability_margins?

@murrayrm
Copy link
MemberAuthor

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).

@murrayrmmurrayrmforce-pushed theuserguide-22Dec2024 branch 2 times, most recently fromd601d38 to68835e4CompareJanuary 21, 2025 05:04
@sawyerbfuller
Copy link
Contributor

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!pip install control and it seems to intelligently cache and not re-download the package every time. Are you sure that the whole
try: import control as ct print("python-control", ct.__version__) except ImportError: !pip install control import control as ct

is necessary in the intro sections? Apologies for bad formatting, I’m typing this on my phone)

@sawyerbfuller
Copy link
Contributor

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:

  • could save an import using np.pi instead of math.pi
  • Reveals a paper cut instepinfo in that many/all of its results are in singleton incarnations of various object types (eg np.float64) rather than plain floats.

@murrayrm
Copy link
MemberAuthor

I’m not an expert in colab but I have often just run it with a!pip install control and it seems to intelligently cache and not re-download the package every time. Are you sure that the wholetry: import control as ct print("python-control", ct.__version__) except ImportError: !pip install control import control as ct

is necessary in the intro sections? Apologies for bad formatting, I’m typing this on my phone)

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
Copy link
Contributor

“ State space models can be manipulated …” appears twice in part 3

@sawyerbfuller
Copy link
Contributor

In part 3,

  • in the TF section, specify that num and den should be arrays of coefficients ordered in decreasing power of s (or z in discrete-time case)
  • in the FRD section, give the units of omega (rad/s) and a little while later examples are specified in terms of freqpts instead, and it’s not clear what the relationship is. Maybe can just use omega in place of freqpts?
  • “.. doctest..” appears in a box after “ A loadable description of a system can be obtained just by displaying the system object:” in sec. 3.6.
  • clicking “next” at the end of 7.4 takes you to a stochastic systems example instead of taking you to chapter 8.

@sawyerbfuller
Copy link
Contributor

Addsample_system to the listing of “ Functions that transform systems from one form to another:” in “function reference”

@murrayrm
Copy link
MemberAuthor

I’m not an expert in colab but I have often just run it with a!pip install control and it seems to intelligently cache and not re-download the package every time. Are you sure that the wholetry: import control as ct print("python-control", ct.__version__) except ImportError: !pip install control import control as ct
is necessary in the intro sections? Apologies for bad formatting, I’m typing this on my phone)

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.

Confirmed that!pip in Colab will not reinstall if it is already there => OK to leave out thetry statement. I'm going to go ahead and leave that in for the tutorial since!pip may not run in all environments (eg, it fails in "raw" python).

@murrayrm
Copy link
MemberAuthor

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:

  • could save an import using np.pi instead of math.pi
  • Reveals a paper cut instepinfo in that many/all of its results are in singleton incarnations of various object types (eg np.float64) rather than plain floats.

I'm updatingstep_info to save everything as afloat. This is a problem a bit more generally, since lots of things returnnp.float64 objects.

@sawyerbfuller
Copy link
Contributor

A few more things as I find them, concentrating on docs:

@murrayrmmurrayrm marked this pull request as ready for reviewJanuary 27, 2025 05:46
@slivingstonslivingston self-requested a reviewJanuary 28, 2025 01:24
@@ -1079,14 +1046,14 @@ def ctrb(A, B, t=None):
Parameters
Copy link
Member

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

Comment on lines 597 to 602
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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

See also2359299 in#1109

Copy link
MemberAuthor

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.

@murrayrmmurrayrm merged commit11d753f intopython-control:mainFeb 3, 2025
49 checks passed
@murrayrmmurrayrm added this to the0.10.2 milestoneFeb 19, 2025
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@bnavigatorbnavigatorbnavigator left review comments

@sawyerbfullersawyerbfullersawyerbfuller left review comments

@slivingstonslivingstonslivingston approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
0.10.2
Development

Successfully merging this pull request may close these issues.

Suggest current best practice of%pip install control
4 participants
@murrayrm@sawyerbfuller@slivingston@bnavigator

[8]ページ先頭

©2009-2025 Movatter.jp