GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — CI tests via GitHub actions and conda by murrayrm · Pull Request #504 · python-control/python-control

Conversation

murrayrm commented Jan 10, 2021 •

edited

Loading

Uh oh!

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

Copy link

Copy Markdown

Member

This is a fairly minimal version of continuous integration (CI) tests using GitHub actions consisting of two workflows:

python-package-conda: This workflow uses a conda-based install for everything and tests only against the latest scipy and Python 3.6 and 3.9. So compared with our current Travis CI testing, we lose scipy-0.19.1 testing and Python 2.7. Coverage is implemented, but not sure (yet) whether it properly connects to coveralls.
control-slycot-src: This workflow installs slycot from source, which allows testing against the latest version of slycot (in case we need it). No coverage testing.

This workflow users coveralls and pushes results up to coveralls.io.

Note: I think that as soon as this PR is merged then we should see the workflows show up in the Actions tab on GitHub. This is working correctly in my personal fork.

Sorry, something went wrong.

Uh oh!

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

All reactions

murrayrm added 4 commits

January 10, 2021 09:06


          Create python-package-conda.yml

3f87c33


          remove flake8 lint; add dependencies

21e47a6


          skip X11 tests if no DISPLAY env variable

d83e67d


          expand test matrix to match Travis CI

627754f

murrayrm linked an issue

Jan 10, 2021

that may be closed by this pull request

Move from legacy Travis CI integration to a Github Actions based system #446

Closed

coveralls commented Jan 10, 2021 •

edited

Loading

Uh oh!

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

Copy link

Copy Markdown

Coverage increased (+11.03%) to 87.677% when pulling 40aac67 on murrayrm:github-actions-conda into a82b520 on python-control:master.

All reactions

Sorry, something went wrong.

Uh oh!

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

bnavigator commented Jan 10, 2021

Copy link

Copy Markdown

Contributor

I am not done with Slycot yet (coming closer), but see https://github.com/bnavigator/Slycot/blob/3c0cb727d5371996c038c94e9bd68621a5430b8a/.github/workflows/slycot-build-and-test.yml for an example how to deal with coveralls.

All reactions

Sorry, something went wrong.

Uh oh!

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

bnavigator reviewed

Jan 10, 2021

View reviewed changes

Comment thread

control/tests/rlocus_test.py Outdated

		roots, kvect = root_locus(sys, plot=False)
		self.check_cl_poles(sys, roots, kvect)

		@X11only

bnavigator Jan 10, 2021 •

edited

Loading

Uh oh!

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

Copy link

Copy Markdown

Contributor

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.

The failures are not due to X11 but because the figure manager has no toolbar. I guess the matplotlib version/setup is different than on the Travis runners.

Same problem has always been present for the openSUSE rpm package, which I maintain. Forcing the PyQt5 backend instead of Agg helped there, but my tries (short) for Slycot on GitHub worklfows did not succeed. I just skip the two tests in python-control/Slycot#140

Sorry, something went wrong.

Uh oh!

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

All reactions

murrayrm Jan 10, 2021

Copy link

Copy Markdown

Member Author

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.

Thanks for the extra info. I'd prefer not to have to factor these out. Let me see if I can use pytest-xvfb or something similar to get it to work.

Sorry, something went wrong.

Uh oh!

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

All reactions

murrayrm Jan 11, 2021

Copy link

Copy Markdown

Member Author

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.

I ended up leaving in these marks since they are useful if you are testing without X11, but also got pytest-xvfb working, so these tests aren't skipped in the GitHub Actions CI testing.

Sorry, something went wrong.

Uh oh!

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

All reactions

bnavigator reviewed

Jan 10, 2021

View reviewed changes

Comment thread

control/tests/conftest.py Resolved

Uh oh!

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

bnavigator reviewed

Jan 10, 2021

View reviewed changes

Comment thread

.github/workflows/python-package-conda.yml Resolved

Uh oh!

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

bnavigator reviewed

Jan 10, 2021

View reviewed changes

Comment thread

.github/workflows/python-package-conda.yml Outdated

Comment on lines +21 to +23

		- uses: actions/checkout@v2
		- name: Set up Python
		uses: actions/setup-python@v2

bnavigator Jan 10, 2021

Copy link

Copy Markdown

Contributor

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.

Conflicts with the conda setup

Sorry, something went wrong.

Uh oh!

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

All reactions

murrayrm Jan 10, 2021

Copy link

Copy Markdown

Member Author

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.

I think I might be able to remove this completely. I meant to do that at one point but lost track of it while playing around to just get something to work. I'll see if I can remove it and if things still work correctly.

Sorry, something went wrong.

Uh oh!

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

All reactions

bnavigator reviewed

Jan 10, 2021

View reviewed changes

Comment thread

.github/workflows/python-package-conda.yml Outdated

Comment on lines +33 to +36

		if [[ '${{matrix.python-version}}' == '2.7' ]]; then
		# matplotlib for Python 2.7 seems to require X11 to run correctly
		sudo apt install -y xvfb
		fi

bnavigator Jan 10, 2021

Copy link

Copy Markdown

Contributor

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.

Why still bother with Python2? Time to ditch it for the next release.

Also, pytest-xvfb is the way to go.

Sorry, something went wrong.

Uh oh!

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

All reactions

murrayrm Jan 10, 2021

Copy link

Copy Markdown

Member Author

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.

Thanks for the suggestion. Will try shifting to pytest-xvfb. I figure we may as well test against Python 2.7 for as long as it continues to work. Amazingly, it is still the default Python on MacOS 11 (Big Sur).

Sorry, something went wrong.

Uh oh!

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

All reactions

bnavigator Jan 10, 2021

Copy link

Copy Markdown

Contributor

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.

If we don't actively remove it, every PR with a failing Py2 test will try to workaround it. I think we should save contributors from this torture.

Sorry, something went wrong.

Uh oh!

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

All reactions

bnavigator reviewed

Jan 10, 2021

View reviewed changes

Comment thread

.github/workflows/python-package-conda.yml Resolved

Uh oh!

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

murrayrm added 2 commits

January 10, 2021 19:43


          simplify GitHub actions workflow

47251c6


          pytest with slycot from source

6ff2e9b

bnavigator reviewed

Jan 11, 2021

View reviewed changes

Comment thread

.github/workflows/python-package-conda.yml Resolved

Uh oh!

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

bnavigator reviewed

Jan 11, 2021

View reviewed changes

Comment thread

.github/workflows/python-package-conda.yml Outdated

Comment on lines +33 to +34

		conda install pip coverage pytest
		conda install -c conda-forge pytest-xvfb pytest-cov

bnavigator Jan 11, 2021

Copy link

Copy Markdown

Contributor

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.

coverage not reported to coveralls yet

Sorry, something went wrong.

Uh oh!

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

All reactions

murrayrm Jan 11, 2021

Copy link

Copy Markdown

Member Author

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.

I think we need to merge the PR before this is run and (hopefully) at that point things start to show up in coveralls?

Sorry, something went wrong.

Uh oh!

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

All reactions

bnavigator Jan 11, 2021

Copy link

Copy Markdown

Contributor

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.

coverage != coveralls.

See https://coveralls-python.readthedocs.io/en/latest/usage/configuration.html#github-actions-gotcha and my previously linked workflow draft for Slycot. (Much more complicated than yours! :( )

Sorry, something went wrong.

Uh oh!

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

All reactions

murrayrm added 4 commits

January 11, 2021 21:15


          update xvfb configuration; remove X11 marks

5bb9de4


          update xvfb configuration; remove X11 marks

36ecea1


          add coveralls

b7b696f


          Merge branch 'github-actions-conda_revised' into github-actions-conda

40aac67

bnavigator reviewed

Jan 12, 2021

View reviewed changes

Comment thread

.github/workflows/python-package-conda.yml

		run: \|
		source $CONDA/bin/activate test-environment
		# Use xvfb-run instead of pytest-xvfb to get proper mpl backend
		# Use coverage instead of pytest-cov to get .coverage file

bnavigator Jan 12, 2021

Copy link

Copy Markdown

Contributor

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.

pytest-cov produces one. See python-control/Slycot#140. Use the --cov parameter.

Sorry, something went wrong.

Uh oh!

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

All reactions

bnavigator reviewed

Jan 12, 2021

View reviewed changes

Comment thread

.github/workflows/python-package-conda.yml

		# Install test tools
		conda install pip coverage pytest
		conda install -c conda-forge pytest-cov
		pip install coveralls

bnavigator Jan 12, 2021

Copy link

Copy Markdown

Contributor

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.

coveralls is on conda-forge nowadays

Sorry, something went wrong.

Uh oh!

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

All reactions

murrayrm commented Jan 12, 2021

Copy link

Copy Markdown

Member Author

I think this PR is good to go. There are some small changes that @bnavigator suggests, but I don't these these substantially add/enhance functionality => we can probably stick with this for now (and see if it all works when merged).

All reactions

Sorry, something went wrong.

Uh oh!

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

murrayrm merged commit 1e6da71 into python-control:master

Jan 12, 2021

bnavigator commented Jan 12, 2021

Copy link

Copy Markdown

Contributor

Sorry, for being so pedantic here. I went through many of these hoops for Slycot and the openSUSE rpm packages so I want to share the "proper" solutions.

The coveralls jobs should get a proper name tag so that the listing looks nicer. (coveralls-python FAQ previously linked)
Compare https://coveralls.io/builds/36275777 to https://coveralls.io/builds/36276758
The coveralls link on the master commit ✔️ is still from Travis. Do we remove Travis eventually or do we try to merge those two coveralls reports per PR/commit, Otherwise we will always have to subsequent datapoints in the history, possibly with different values.
The way miniconda is set up this limits everything to Unix. Maybe even Linux only.

All reactions

Sorry, something went wrong.

Uh oh!

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

murrayrm commented Jan 12, 2021

Copy link

Copy Markdown

Member Author

I'll move these comments to issue #446 so that we can continue to track these improvements. I mainly wanted to get shifted over to some new test infrastructure since Travis CI is getting so frustrating. (GitHub actions clears commits in 2-3 minutes, versus what was often 30+ min for Travis).

I think we can leave Travis and GitHub Actions running for a few days, as we see whether new PRs and things are properly handled, but then I think we shift away from Travis completely.

bnavigator reacted with thumbs up emoji

All reactions

👍 1 reaction

Sorry, something went wrong.

Uh oh!

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

murrayrm mentioned this pull request

Jan 12, 2021

Move from legacy Travis CI integration to a Github Actions based system #446

Closed

murrayrm deleted the github-actions-conda branch

January 14, 2021 07:04

murrayrm mentioned this pull request

Jan 14, 2021

Release 0.8.4 and 0.9.0 sequencing #451

Closed

murrayrm added this to the 0.9.0 milestone

Mar 20, 2021

bnavigator mentioned this pull request

Aug 4, 2022

CI tests are failing (xvfb issue?) #756

Closed

Reviewers

bnavigator

bnavigator left review comments

Uh oh!

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

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

0.9.0

Development

Successfully merging this pull request may close these issues.

Move from legacy Travis CI integration to a Github Actions based system

Uh oh!

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

3 participants

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.

Conversation

murrayrm commented Jan 10, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coveralls commented Jan 10, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bnavigator commented Jan 10, 2021

Uh oh!

bnavigator Jan 10, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

murrayrm commented Jan 12, 2021

Uh oh!

bnavigator commented Jan 12, 2021

Uh oh!

murrayrm commented Jan 12, 2021

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

murrayrm commented Jan 10, 2021 •

edited

Loading

Uh oh!

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

coveralls commented Jan 10, 2021 •

edited

Loading

Uh oh!

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

bnavigator Jan 10, 2021 •

edited

Loading

Uh oh!

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