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 — Add run of tests without Slycot on Travis CI by slivingston · Pull Request #133 · python-control/python-control

Conversation

@slivingston

@slivingston slivingston commented Feb 17, 2017

Copy link
Copy Markdown
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.

Sorry, something went wrong.

Uh oh!

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

All reactions
slivingston added 2 commits February 17, 2017 11:14
@slivingston
TEST: Add run of tests without Slycot on TravisCI
1757ff0
@slivingston
TEST: Disable tests that depend on Slycot if absent
a7e0bba
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.
@slivingston slivingston added enhancement quality assurance labels Feb 17, 2017
@coveralls

coveralls commented Feb 17, 2017 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown

Coverage Status

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

All reactions

Sorry, something went wrong.

Uh oh!

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

@slivingston

slivingston commented Feb 17, 2017

Copy link
Copy Markdown
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.

All reactions

Sorry, something went wrong.

Uh oh!

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

@roryyorke

roryyorke commented Feb 17, 2017

Copy link
Copy Markdown
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.
All reactions

Sorry, something went wrong.

Uh oh!

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

@murrayrm

murrayrm commented Feb 18, 2017

Copy link
Copy Markdown
Member

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

All reactions

Sorry, something went wrong.

Uh oh!

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

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

murrayrm commented Feb 20, 2017

Copy link
Copy Markdown
Member

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

All reactions

Sorry, something went wrong.

Uh oh!

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

@slivingston
slivingston deleted the testing branch March 4, 2017 04:58
@slivingston

slivingston commented Mar 4, 2017

Copy link
Copy Markdown
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.

All reactions

Sorry, something went wrong.

Uh oh!

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

@kwlee2025cpp kwlee2025cpp mentioned this pull request Jun 26, 2026
Add scipy fallbacks for generalized Lyapunov and discrete Sylvester equations #1234
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Uh oh!

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

Assignees

@murrayrm murrayrm

Labels

enhancement quality assurance

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

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

4 participants

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