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 — ctrb treats ndim=1 B correctly; ctrb & obsv check input shapes by roryyorke · Pull Request #1099 · python-control/python-control

Conversation

@roryyorke

@roryyorke roryyorke commented Jan 19, 2025

Copy link
Copy Markdown
Contributor

I also fixed easy errors pointed out by pyflakes in the two files I changed; these remain. The first one's obviously a bug, I'll open an issue. Not sure about the second two, probably just oversights?

Fixes gh-1097.

control/statefbk.py:278:5: local variable 'b' is assigned to but never used
control/tests/statefbk_test.py:857:13: local variable 'Ki' is assigned to but never used
control/tests/statefbk_test.py:1237:5: local variable 'points' is assigned to but never used

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 08:21
@roryyorke
ctrb converts ndim=1 B correctly; ctrb & obsv check input shapes
236c1d4
@roryyorke
Easy pyflakes fixes in statefbk.py and statefbk_test.py
465cfad
@roryyorke roryyorke mentioned this pull request Jan 19, 2025
statefbk.acker doesn't use _ssmatrix-converted value of inputs #1100
Closed
@coveralls

coveralls commented Jan 19, 2025 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown

Coverage Status

coverage: 94.69% (+0.003%) from 94.687%
when pulling b13ade4 on roryyorke:gh1097-ctrb-obsv-shapes
into 0ff0452 on python-control:main.

All reactions

Sorry, something went wrong.

Uh oh!

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

@murrayrm

murrayrm commented Jan 19, 2025

Copy link
Copy Markdown
Member

LGTM. Will merge in a bit (just in case someone else wants to take a look).

All reactions

Sorry, something went wrong.

Uh oh!

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

@slivingston
slivingston self-requested a review January 20, 2025 03:00
slivingston
slivingston approved these changes Jan 20, 2025
View reviewed changes

@slivingston slivingston left a comment

Copy link
Copy Markdown
Member

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.

Code is correct. I found a misprint in the test comments. If I simply misinterpreted the comment, feel free to merge.

Sorry, something went wrong.

Uh oh!

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

All reactions
Comment thread control/tests/statefbk_test.py Resolved

Uh oh!

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

@roryyorke

roryyorke commented Jan 20, 2025 via email

Copy link
Copy Markdown
Contributor Author
The suggested change is right, tha ks.
…
On Mon, 20 Jan 2025, 05:20 Scott C. Livingston, ***@***.***> wrote: ***@***.**** approved this pull request. Code is correct. I found a misprint in the test comments. If I simply misinterpreted the comment, feel free to merge. ------------------------------ In control/tests/statefbk_test.py <#1099 (comment)> : > @@ -79,6 +96,23 @@ def testObsvT(self): Wo = obsv(A, C, t=t) np.testing.assert_array_almost_equal(Wo, Wotrue) + def testObsvNdim1(self): + # gh-1097: treat 1-dim C as 1xn + A = np.array([[1., 2.], [3., 4.]]) + C = np.array([5., 7.]) + Wotrue = np.array([[5., 7.], [26., 38.]]) + Wo = obsv(A, C) + np.testing.assert_array_almost_equal(Wo, Wotrue) + + def testObsvRejectMismatch(self): + # gh-1097: check A, B for compatible shapes ⬇️ Suggested change - # gh-1097: check A, B for compatible shapes + # gh-1097: check A, C for compatible shapes — Reply to this email directly, view it on GitHub <#1099 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAA3C7QM7AIWRRKUS62XJE32LRTQHAVCNFSM6AAAAABVOMBZOOVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDKNRRGIZTMMRXHE> . You are receiving this because you authored the thread.Message ID: ***@***.***>
All reactions

Sorry, something went wrong.

Uh oh!

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

@murrayrm @slivingston
Update control/tests/statefbk_test.py
b13ade4
Co-authored-by: Scott C. Livingston <[email protected]>
@murrayrm
murrayrm merged commit bddc796 into python-control:main Jan 20, 2025
@murrayrm murrayrm added this to the 0.10.2 milestone Feb 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@slivingston slivingston slivingston 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.

Unexpected and unwanted behaviour of control.ctrb

Uh oh!

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

4 participants

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