GV

GeezerVison

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

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Lint fixes on benchmarks and examples/*.py by roryyorke · Pull Request #1135 · python-control/python-control

Conversation

@roryyorke

@roryyorke roryyorke commented Mar 2, 2025 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Contributor

I don't think these are too controversial.

CI failures

I see two failures on CI , but I can't see that these changes caused them:

first, in doctest-linux: https://github.com/roryyorke/python-control/actions/runs/13612156165/job/38050709282

Traceback
=========

      File "/home/runner/miniconda3/envs/doctest-env/lib/python3.12/site-packages/sphinx/events.py", line 415, in emit
        raise ExtensionError(
    sphinx.errors.ExtensionError: Handler <function html_collect_pages at 0x7f36a35016c0> for event 'html-collect-pages' threw an exception (exception: module 'sphinx.util' has no attribute 'console')

which seems to be sphinx-doc/sphinx#13352.

Then in install-examples: https://github.com/roryyorke/python-control/actions/runs/13612156159/job/38050709274

ModuleNotFoundError: No module named 'autoreload'

I'll install jupyterlab in my dev environment and see if I can reproduce this. I can't see that changing *.py files in examples could cause this.

tfvis.py and Pwm Pmw

When I ran tfvis.py under Python 3.12, I got

AttributeError: module 'collections' has no attribute 'Callable'

in Pwm Pmw code.

Callable was moved to collections.abc in Python 3.10 (October 2021). The hack below lets the script run, but unless pwm Pmw is updated we should consider retiring this example.

modified   examples/tfvis.py
@@ -48,6 +48,9 @@ from matplotlib.backends.backend_tkagg import FigureCanvasTkAgg
 from control.matlab import logspace
 from numpy import array, conj, polymul, real, size
 
+import collections.abc
+collections.Callable = collections.abc.Callable
+
 
 def make_poly(facts):
     """ Create polynomial from factors """

Warnings in examples

Some examples raise control deprecation warnings, e.g., genswitch.py uses phase_plot instead of phase_plane_plot. I started looking into this, but it got a bit involved.

Lint on notebooks

ruff can also run on Jupyter notebooks. Some fixes are obvious:

examples/bode-and-nyquist-plots.ipynb:cell 2:2:17: F401 [*] `scipy` imported but unused
  |
1 | import numpy as np
2 | import scipy as sp
  |                 ^^ F401
3 | import matplotlib.pyplot as plt
4 | import control as ct
  |
  = help: Remove unused import: `scipy`

but I'm less sure about changing these * imports; they are convenient, and make things more familiar for Matlab users:

examples/pvtol-lqr-nested.ipynb:cell 3:2:1: F403 `from matplotlib.pyplot import *` used; unable to detect undefined names
  |
1 | from numpy import *             # Grab all of the NumPy functions
2 | from matplotlib.pyplot import * # Grab MATLAB plotting functions
  | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ F403
3 | from control.matlab import *    # MATLAB-like functions
4 | %matplotlib inline
  |

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
roryyorke added 2 commits March 1, 2025 16:49
@roryyorke
Fix lint warnings in benchmarks/
223c38f
@roryyorke
Lint fixes to examples/*.py
afd3fe3
@murrayrm

murrayrm commented Mar 2, 2025

Copy link
Copy Markdown
Member

The CI errors are also showing up in #1133, so probably something changed on the GitHub Actions end of things. I'll set up a new issue to track that.
All reactions

Sorry, something went wrong.

Uh oh!

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

coveralls commented Mar 2, 2025 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown

Coverage Status

coverage: 94.745%. remained the same
when pulling 5653531 on roryyorke:rory/lint-examples
into ad996f9 on python-control:main.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@murrayrm murrayrm mentioned this pull request Mar 2, 2025
CI failures in doctest, examples #1136
Closed
@roryyorke

roryyorke commented Mar 2, 2025

Copy link
Copy Markdown
Contributor Author

The CI errors are also showing up in #1133, so probably something changed on the GitHub Actions end of things.

Thanks. I can't reproduce the notebook conversion failure locally. I've attached the output of mamba list to record versions of everything installed.

I see there's already a bug on Pmw for collections.Callable, filed in 2020: https://sourceforge.net/p/pmw/bugs/52/

packages.txt
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@murrayrm murrayrm added this to the 0.10.2 milestone Mar 3, 2025
@murrayrm

murrayrm commented Mar 8, 2025

Copy link
Copy Markdown
Member

Sorry for the delay in reviewing this. All of the changes look good to me.

Some comments on your comments:

tfvis.py and Pmw

Callable was moved to collections.abc in Python 3.10 (October 2021). The hack below lets the script run, but unless pwm Pmw is updated we should consider retiring this example.

I think it would be fine to move this to examples/legacy. To a large extent the functionality demonstrated in the script is captured in sisotool.

Warnings in examples

Some examples raise control deprecation warnings, e.g., genswitch.py uses phase_plot instead of phase_plane_plot. I started looking into this, but it got a bit involved.

Perhaps flag some of these as separate issues? For the specific case of genswitch.py (and perhaps others), I likely have new versions that I did for FBS2e (I'm slowly converting all examples and figures to use python-control).

Lint on notebooks

... I'm less sure about changing these * imports; they are convenient, and make things more familiar for Matlab users ...

It would be nice to run lint on the notebooks. I agree that for MATLAB users, it will be convenient to use from control.matlab import * and perhaps we should just flag that one as OK wherever it appears. But I believe that the main control toolbox should always use a prefix (preferably ct for consistency).
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
murrayrm
murrayrm approved these changes Mar 8, 2025
View reviewed changes
@roryyorke
Merge remote-tracking branch 'upstream/main' into rory/lint-examples
5653531
@murrayrm
murrayrm merged commit 394e1c2 into python-control:main Mar 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@murrayrm murrayrm murrayrm approved these changes

Uh oh!

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

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

0.10.2

Development

Successfully merging this pull request may close these issues.

Uh oh!

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

3 participants

@roryyorke @murrayrm @coveralls
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.