Skip to content

Add run of tests without Slycot on Travis CI #133

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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Feb 20, 2017
Merged

Add run of tests without Slycot on Travis CI #133

merged 2 commits into from
Feb 20, 2017

Conversation

slivingston
Copy link
Member

Because tests and parts of control that are used depend on whether Slycot is installed, it is worthwhile to exercise both cases as part of CI testing. This pull request does so. Test coverage is only for the case with Slycot installed (as from before this pull request).

The second commit disables tests that depend on Slycot if it is not found. Affected tests were introduced in PR #118.

testBalredMatchDC, testGramRc, testGramRo
which were introduced in PR #118
use functions that depend on Slycot,
so the tests should be skipped if it is not found.
@coveralls
Copy link

coveralls commented Feb 17, 2017

Coverage Status

Coverage remained the same at 77.329% when pulling a7e0bba on testing into 5ab74cf on master.

@slivingston
Copy link
Member Author

Changes here increase runtime for CI builds by approximately 90 seconds (as a sum across all configurations: Python 2.7, 3.3, 3.4, 3.5).

There is at least one negative consequence of changes here. Tests that are independent of Slycot are performed twice without potential benefit. However, the marginal gain in computation time and load is small.

@roryyorke
Copy link
Contributor

Looks good. I assume you saw all the deprecation warnings from scipy.

FWIW, an alternative is to use Travis CI's matrix capability; I imagine "with Slycot" and "without Slycot" configurations. Using a matrix setup opens other possibilities:

  1. a "no-conda" config---I don't know how useful this is, but I gather from express dependencies in setup.py Slycot#3 that this could be desired.
  2. Mac OS testing. I haven't explored this (don't have a Mac), but I think Travis CI does support it.

@murrayrm
Copy link
Member

This PR looks good to me. Will merge in 24 hours if there are no further comments.

@murrayrm murrayrm self-assigned this Feb 18, 2017
@murrayrm murrayrm merged commit 6ada795 into master Feb 20, 2017
@murrayrm
Copy link
Member

@slivingston: leaving it to you to delete the branch (just in case there is some reason to keep it).

@slivingston slivingston deleted the testing branch March 4, 2017 04:58
@slivingston
Copy link
Member Author

@roryyorke and @murrayrm thanks to both of you for reviews.

re #133 (comment):

  1. Yes, I noticed the deprecation warnings, but I think that it is outside the scope of this pull request. My plan is to add more tests while addressing issue Organize unit tests to follow the NumPy/SciPy guidelines #95 before making changes to avoid deprecated external API.

  2. Before opening this PR, I tried testing on Mac OS as part of a matrix setup on Travis CI, e.g., https://travis-ci.org/python-control/python-control/builds/201547383. Ultimately I did not succeed. Because Slycot is the main difficulty in terms of cross-platform support, I decided to devote more effort to configuring Mac OS on Travis CI for Slycot instead of control.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants