GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — Fix or silence `ruff check` warnings; add `ruff check` to Github Actions by roryyorke · Pull Request #246 · python-control/Slycot

Conversation

roryyorke commented Jan 19, 2025

Copy link

Copy Markdown

Collaborator

Is this worth it at all?

Are the particular fixes I've made (especially # noqa ones) reasonable?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

roryyorke added 2 commits

January 19, 2025 11:52


          Run "ruff check" on source in Github action

f305cc5


          Make source ruff check-clean

Removed unused imports.

Removed unused variables in test: these were cut-and-paste from other
tests, and not needed for the tests in questions.

Other minor fixes.

roryyorke requested a review from bnavigator

January 19, 2025 11:27

roryyorke mentioned this pull request

Jan 19, 2025

pyflakes (pylint, ruff, ....) clean-up, CI? python-control/python-control#1101

Open

bnavigator commented Jan 30, 2025

Copy link

Copy Markdown

Collaborator

I like the general approach! Thanks @roryyorke!

I would prefer not to override E741 (ambigous variable name) on single lines. I'd rather just switch it off globally. We use the single letter variable names where SLICOT uses them.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

bnavigator requested changes

Jan 30, 2025

View reviewed changes

Comment thread

slycot/analysis.py Outdated



		def ag08bd(l,n,m,p,A,E,B,C,D,equil='N',tol=0.0,ldwork=None):
		def ag08bd(l,n,m,p,A,E,B,C,D,equil='N',tol=0.0,ldwork=None): # noqa: E741

bnavigator Jan 30, 2025

Copy link

Copy Markdown

Collaborator

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.

Just switch off E741 in a global option in pyproject.toml

[tool.ruff.lint]
ignore = [ "E741" ]

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

roryyorke Feb 1, 2025

Copy link

Copy Markdown

Collaborator Author

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.

Done. If it weren't backward incompatible I'd be tempted to change l to ell, but so it goes.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions


          Disable Ruff 3741 project-wide

f1cb889

roryyorke commented Feb 1, 2025

Copy link

Copy Markdown

Collaborator Author

From what I see, all failures are on macos, and I think all macos jobs fail. I checked two at random, and the failure is in python-control TestStateSpace.test_pow:

___________________________ TestStateSpace.test_pow ____________________________
  
  self = <control.tests.statesp_test.TestStateSpace object at 0x10b791ff0>
  sys222 = StateSpace(
  array([[ 4.,  1.],
         [ 2., -3.]]),
  array([[ 5.,  2.],
         [-3., -3.]]),
  array([[ 2., -4.],
         [ 0.,  1.]]),
  array([[ 3.,  2.],
         [ 1., -1.]]),
  states=2, outputs=2, inputs=2)
  sys322 = StateSpace(
  array([[-3.,  4.,  2.],
         [-1., -3.,  0.],
         [ 2.,  5.,  3.]]),
  array([[ 1.,  4.],
         [-3., ..., -3.],
         [ 1.,  4.,  3.]]),
  array([[-2.,  4.],
         [ 0.,  1.]]),
  name='sys322', states=3, outputs=2, inputs=2)
  
      @slycotonly
      def test_pow(self, sys222, sys322):
          """Test state space powers."""
          for sys in [sys222, sys322]:
              # Power of 0
              result = sys**0
              expected = StateSpace([], [], [], np.eye(2), dt=0)
              np.testing.assert_allclose(expected.A, result.A)
              np.testing.assert_allclose(expected.B, result.B)
              np.testing.assert_allclose(expected.C, result.C)
              np.testing.assert_allclose(expected.D, result.D)
              # Power of 1
              result = sys**1
              expected = sys
              np.testing.assert_allclose(expected.A, result.A)
              np.testing.assert_allclose(expected.B, result.B)
              np.testing.assert_allclose(expected.C, result.C)
              np.testing.assert_allclose(expected.D, result.D)
              # Power of -1 (inverse of biproper system)
              # Testing transfer function representations to avoid the
              # non-uniqueness of the state-space representation. Once MIMO
              # canonical forms are supported, can check canonical state-space
              # matrices instead.
              result = (sys * sys**-1).minreal()
              expected = StateSpace([], [], [], np.eye(2), dt=0)
              assert _tf_close_coeff(
                  ss2tf(expected).minreal(),
                  ss2tf(result).minreal(),
              )
              result = (sys**-1 * sys).minreal()
              expected = StateSpace([], [], [], np.eye(2), dt=0)
  >           assert _tf_close_coeff(
                  ss2tf(expected).minreal(),
                  ss2tf(result).minreal(),
              )
  E           assert False
  E            +  where False = _tf_close_coeff(TransferFunction(\n[[array([1.]), array([0.])],\n [array([0.]), array([1.])]],\n[[array([1.]), array([1.])],\n [array([1.]), array([1.])]],\noutputs=2, inputs=2), TransferFunction(\n[[array([1.]), array([-2.76395297e-15, -1.35541257e-13,  9.01576541e-13])],\n [array([0.]), array([1.])]],\n[[array([1.]), array([  1.,   3.,  -9., -41.])],\n [array([1.]), array([1.])]],\noutputs=2, inputs=2))
  E            +    where TransferFunction(\n[[array([1.]), array([0.])],\n [array([0.]), array([1.])]],\n[[array([1.]), array([1.])],\n [array([1.]), array([1.])]],\noutputs=2, inputs=2) = minreal()
  E            +      where minreal = TransferFunction(\n[[array([1.]), array([0.])],\n [array([0.]), array([1.])]],\n[[array([1.]), array([1.])],\n [array([1.]), array([1.])]],\noutputs=2, inputs=2).minreal
  E            +        where TransferFunction(\n[[array([1.]), array([0.])],\n [array([0.]), array([1.])]],\n[[array([1.]), array([1.])],\n [array([1.]), array([1.])]],\noutputs=2, inputs=2) = ss2tf(StateSpace(\narray([], shape=(0, 0), dtype=float64),\narray([], shape=(0, 2), dtype=float64),\narray([], shape=(2, 0), dtype=float64),\narray([[1., 0.],\n       [0., 1.]]),\nstates=0, outputs=2, inputs=2))
  E            +    and   TransferFunction(\n[[array([1.]), array([-2.76395297e-15, -1.35541257e-13,  9.01576541e-13])],\n [array([0.]), array([1.])]],\n[[array([1.]), array([  1.,   3.,  -9., -41.])],\n [array([1.]), array([1.])]],\noutputs=2, inputs=2) = minreal()
  E            +      where minreal = TransferFunction(\n[[array([  1.,   3.,  -9., -41.]), \n  array([-2.76395297e-15, -1.35541257e-13,  9.01576541e-13])],\n ....,   3.,  -9., -41.]), array([  1.,   3.,  -9., -41.])],\n [array([1.]), array([1., 0., 0., 0.])]],\noutputs=2, inputs=2).minreal
  E            +        where TransferFunction(\n[[array([  1.,   3.,  -9., -41.]), \n  array([-2.76395297e-15, -1.35541257e-13,  9.01576541e-13])],\n ....,   3.,  -9., -41.]), array([  1.,   3.,  -9., -41.])],\n [array([1.]), array([1., 0., 0., 0.])]],\noutputs=2, inputs=2) = ss2tf(StateSpace(\narray([[  4.86563613,  -0.22114886, -14.38268526],\n       [  3.86388385,  -1.64824725, -16.2652334 ],\n    ... [0.00000000e+00, 2.59786829e-16, 3.85014366e-15]]),\narray([[1., 0.],\n       [0., 1.]]),\nstates=3, outputs=2, inputs=2))
  
  control/tests/statesp_test.py:596: AssertionError

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

roryyorke commented Feb 1, 2025

Copy link

Copy Markdown

Collaborator Author

not all macos tests fail; many pass, but 7 fail.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

bnavigator commented Feb 1, 2025

Copy link

Copy Markdown

Collaborator

Failing macOS tests are unrelated. Thanks @roryyorke

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

bnavigator merged commit bbfe718 into python-control:master

Feb 1, 2025

roryyorke mentioned this pull request

Sep 7, 2025

Release 0.6.1 #248

Closed

Reviewers

bnavigator

bnavigator requested changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

2 participants

Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.