Skip to content

Fix name errors in statesp module#192

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

Fix name errors in statesp module#192
Sup3rGeo wants to merge 1 commit intopython-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
Copy link
Copy Markdown

coveralls commented Feb 19, 2018

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