Skip to content

Fix name errors in statesp module - #192

Closed
Sup3rGeo wants to merge 1 commit into
python-control:masterfrom
Sup3rGeo:fix-statesp-pi
Closed

Sup3rGeo wants to merge 1 commit into
python-control:masterfrom
Sup3rGeo:fix-statesp-pi

Conversation

@Sup3rGeo

Copy link
Copy Markdown

No description provided.

@coveralls

Copy link
Copy Markdown

Coverage Status

Coverage remained the same at 77.882% when pulling dc8410c on Sup3rGeo:fix-statesp-pi into 601b581 on python-control:master.

2 similar comments
@coveralls

Copy link
Copy Markdown

Coverage Status

Coverage remained the same at 77.882% when pulling dc8410c on Sup3rGeo:fix-statesp-pi into 601b581 on python-control:master.

@coveralls

coveralls commented Feb 19, 2018 •

Copy link
Copy Markdown

Coverage Status

Coverage remained the same at 77.882% when pulling dc8410c on Sup3rGeo:fix-statesp-pi into 601b581 on python-control:master.

@murrayrm

Copy link
Copy Markdown
Member

It looks like this slipped through because there was no unit test that executed this part of the code. Can we add a unit test that covers this section of code (and hence will prevent further errors of this sort)?

@murrayrm

Copy link
Copy Markdown
Member

I've put together a unit test that exposes this error and also found another small bug in the way the test for frequencies above the Nyquist frequency are handled. I'll generate a separate pull request with the changes.

murrayrm added a commit to murrayrm/python-control that referenced this pull request Feb 24, 2018
As pointed out in PR python-control#192, there were some namespace errors in the statesp
module related to checking whether the frequency range as valid for discrete
time systems.  I added a unit test that covered this code (and exposed the
error) and fixed up the namespace problems (as in PR python-control#192).  There was also
a logic error in the way that frequencies were checked and the warning message
referred to the wrong function.
@murrayrm

Copy link
Copy Markdown
Member

Superseded by PR #196.

@murrayrm murrayrm closed this Feb 24, 2018
@Sup3rGeo
Sup3rGeo deleted the fix-statesp-pi branch May 24, 2018 08:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants