Skip to content

Update docstrings for LTI class constructors#171

Merged
murrayrm merged 7 commits into
python-control:masterfrom
murrayrm:statesp_constructor_doc
Jan 2, 2018
Merged

Update docstrings for LTI class constructors#171
murrayrm merged 7 commits into
python-control:masterfrom
murrayrm:statesp_constructor_doc

Conversation

@murrayrm

Copy link
Copy Markdown
Member

This PR addresses the documentation issues raised in PR #163 related to how the StateSpace, TransferFunction, and FRD constructors are documented. In this PR, the first line of the docstring is modified for the each of the above classes, as well as their init methods and the corresponding ss, tf, and frd functions to have the most common call signature.

This PR leaves in place the copy constructor functionality and only modifies the documentation.

While I was at it, I also fixed some indentation problems and other issues that were causing warnings in sphinx.

@murrayrm

Copy link
Copy Markdown
Member Author

Added more information to main documentation on class constructors + updated docstrings for other functions with variable arguments. The justification for including them in this PR is that they are consistent with the initial changes and the comments in PR #163.

Comment thread doc/conventions.rst Outdated

.. math::

G(s) = \frac{\text{num}(s)}{\text{den(s)}}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

in first case (s) is outside \text{}, in second case inside it.

Comment thread doc/conventions.rst Outdated
Linear time invariant (LTI) systems are represented in python-control in
state space, transfer function, or frequency response data (FRD) form. Most
functions in the toolbox will operate on any of these data types and
functions for convering between between compatible types is provided.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

converting

Comment thread control/margins.py Outdated

def margin(*args):
"""Calculate gain and phase margins and associated crossover frequencies
"""margin(sys)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Just below (line 345) the name "sysdata" is used; perhaps that should be used here?

Comment thread control/ctrlutil.py Outdated
# Hack for sphinx.ext.autodoc: if numpy is a mock import, then numpy.pi
# will be assigned to _Mock() and this generates a type error
if not isinstance(pi, float):
pi = 3.14

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

how about "from math import pi" here? Or does that result in a mocked instance of some sort too?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Much better fix since math is included in the base python distribution.

Comment thread doc/conventions.rst Outdated
= \frac{a_0 s^n + a_1 s^{n-1} + \cdots + a_n}
{b_0 s^m + b_1 s^{m-1} + \cdots + b_m},

where n is generally greater than m (for a proper transfer function).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

maybe greater than or equal to?

@murrayrm murrayrm mentioned this pull request Dec 30, 2017
@murrayrm

Copy link
Copy Markdown
Member Author

Good feedback @roryyorke. Small updates pushed to address these.

@murrayrm murrayrm merged commit f890a88 into python-control:master Jan 2, 2018
@murrayrm murrayrm deleted the statesp_constructor_doc branch January 2, 2018 16:13
@murrayrm murrayrm mentioned this pull request Jan 6, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants