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

Lint tests#1127

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 24 commits intopython-control:mainfromroryyorke:lint-tests
Feb 19, 2025
Merged

Lint tests#1127

murrayrm merged 24 commits intopython-control:mainfromroryyorke:lint-tests
Feb 19, 2025

Conversation

@roryyorke
Copy link
Contributor

This extends ruff testing to control/tests/*.py

I've tried to order the commits from obvious to subtle; on the whole:

  • remove unused imports
  • removed unused variables, or sometimes prefix with '_' where that seemed to make things clearer
  • tweaked twoeval calls

The remaining errors I'm less sure of. It would be simple to silence them with# noqa: F<relevant code>, and defer fixing these to later; then at least no new ruff errors will sneak in in the future.

More detail on remaining failures:

control/tests/kwargs_test.py

Here the dict literal has repeated keys for'StateSpace.__init__' and'StateSpace.__sample':

    'StateSpace.__init__':        interconnect_test.test_interconnect_exceptions,    'StateSpace.sample': test_unrecognized_kwargs,    'NonlinearIOSystem.__init__':        interconnect_test.test_interconnect_exceptions,    'StateSpace.__init__': test_unrecognized_kwargs,    'StateSpace.initial_response': timeresp_test.test_timeresp_aliases,    'StateSpace.sample': test_unrecognized_kwargs,

Last-value-wins in dict literals, so the first two entries have no effect. It looks like this is just a check thatsomething exists, so I guess we can just erase the first two entries?

control/tests/modelsimp_test.py

The variablemrk is assigned to:mrk = markov(outp, inp, 1, transpose=False). I didn't investigate issue#395 referenced above -- is it enough that this call doesn't raise?

control/tests/optimal_test.py

curved_seg_length is calculated, but not used. It's not obviously copy-and-pasted from another test, so I'm not sure if the test should be changed here. It's obviously easy enough to just remove this variable, andstraight_seg_length just above it.

Where it's clear that function is called for side effects (e.g., in aq`with pytest.raises` block), don't assign function output.  Whereit's not clear, e.g., binary ops on LTI objects, call result `_sys` orsimilar.  There are plenty of in-between cases: for those I chosebased on understandability.
Where imports were test fixtures, replaced with`@pytest.mark.usefixtures('nameoffixture')`.
Provide relevant symbols via `locals` argument to eval.
Symbol reference in lambda which is never called.
The test had no assertions, but have been intended to test MIMOfeedback; for this see testMIMOfb in same file.
The function doesn't exist, but the test is never called.
It looks like a block of code was copied-and-pasted between two testfunctions; removed the unused variable in each case.
@roryyorkeroryyorke marked this pull request as draftFebruary 16, 2025 10:42
Only works from Python 3.13.
@coveralls
Copy link

coveralls commentedFeb 16, 2025
edited
Loading

Coverage Status

coverage: 94.744%. remained the same
when pullingc907a4f on roryyorke:lint-tests
into2a919cc on python-control:main.

@murrayrm
Copy link
Member

control/tests/kwargs_test.py

Correct that there just needs to be some test that verifies that unknown keywords generate an error. Whichever of the tests that has that check should be kept and if both do then keeping either is fine.

control/tests/modelsimp_test.py

Correct that the intent of this test was just to make sure an exception wasn't raised. We could either get rid of the assignment tomrk or check the output to make sure it does something sensible. For example:

np.testing.assert_almost_equal(mrk, 2.)

control/tests/optimal_test.py

I'm pretty sure I wrote this example and I can't see any reason to keepstraight_seg_length orcurved_seg_length.

@roryyorkeroryyorke marked this pull request as ready for reviewFebruary 16, 2025 17:45
@roryyorkeroryyorke changed the title[WIP] Lint testsLint testsFeb 16, 2025
@roryyorke
Copy link
ContributorAuthor

Oops - had a commit there I'd intended for another PR. Reverted.

If this PR isn't merged now, it'll be worth merging main into the PR before merging the PR to main, to make sure the lint checks don't causes failures on main after the merge of the PR. (I hope that sentence is less confusing than I think it is!)

@murrayrmmurrayrm merged commitf6799ab intopython-control:mainFeb 19, 2025
24 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

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

0.10.2

Development

Successfully merging this pull request may close these issues.

3 participants

@roryyorke@coveralls@murrayrm

[8]ページ先頭

©2009-2025 Movatter.jp