Update _ssmatrix and _check_shape for consistent usage by murrayrm · Pull Request #1116 · python-control/python-control

@murrayrm

Prompted by issue #1100, this PR updates the _ssmatrix and _check_shape functions to make their usage more consistent and remove legacy calls that are no longer needed.

Changes:

  • Updated _ssmatrix to allow checking for square matrices and row/col dimensions, with improved error messages that include the name of the matrix generating the error.
  • Functions that accept state-space matrices now call _ssmatrix for uniform processing.
  • Removed unneeded calls to _ssmatrix on outputs of functions in mateqn.py and other places. These calls were used back when we supported the NumPy matrix class and are no longer required.
  • Fixed acker to use the matrices generated by _ssmatrix.
  • Updated _check_shape to have a call signature that is more conistent with _ssmatrix: the matrix and dimensions are first, followed by optional keywords, with a name keyword for the name of the matrix being checked (for error messages).
  • Removed internal _check_shape in statefbk.py and replaced with a call to mateqn._check_shape.
  • Updated unit tests to reflect the new wording of some error messages.

@murrayrm

@murrayrm murrayrm linked an issue

Feb 2, 2025

that may be closed by this pull request

@coveralls

Coverage Status

coverage: 94.661% (+0.002%) from 94.659%
when pulling 4fc70f7 on murrayrm:update_ssmatrix-01Feb2025
into 2cb0520 on python-control:main.

roryyorke

control.dlqe: '9db995ed95c2214ce97074b0616a3191',
control.dlqr: '896cfa651dbbd80e417635904d13c9d6',
control.lqe: '567bf657538935173f2e50700ba87168',
control.dlqe: 'f2e52e35692cf5ffe911684d41d284c9',

Choose a reason for hiding this comment

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

I think these hashes are new-ish, just got caught out by them working on adding ruff-pyflakes checks. Is the idea to require the developer changing code to consider implications on the docstring? Or, I suppose, vice versa.

Choose a reason for hiding this comment

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

Yes, that was the idea. However, they will go away when #1094 is merged (I found a better way to handle them).