GeezerVison

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

GeezerVison is showing github.com — GitHub workflow by bnavigator · Pull Request #140 · python-control/Slycot

Conversation

bnavigator commented Dec 30, 2020 •

edited

Loading

Uh oh!

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

Copy link

Copy Markdown

Collaborator

See https://github.com/bnavigator/Slycot/tree/github-workflow for the GitHub Actions results.

Tasks:

Ubuntu
- setup.py install
- Conda
- Pip
macOS
- Conda
- Pip
Windows
- Conda
Python Versions: 3.6 to 3.9
Streamline LAPACK linking. The conda-forge packages nowadays always link against the reference implementation but you can use a different one (=the same as numpy) during runtime.
do Add CI test for import slycot; slycot.test #137 : Test slycot.test()

Builds on #143

Sorry, something went wrong.

Uh oh!

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

All reactions

bnavigator mentioned this pull request

Dec 30, 2020

Consider switching CI to Github Workflows, include Windows #108

Closed

bnavigator linked an issue

Dec 30, 2020

that may be closed by this pull request

Consider switching CI to Github Workflows, include Windows #108

Closed

roryyorke reviewed

Dec 30, 2020

View reviewed changes

Comment thread

conda-recipe-generic/build.sh Outdated

		@@ -0,0 +1,3 @@
		$PYTHON setup.py build_ext install -- \
		-DNumPy_INCLUDE_DIR=${SP_DIR}/numpy/core/include \

roryyorke Dec 30, 2020

Copy link

Copy Markdown

Collaborator

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 just filed scikit-build/scikit-build#524 because I think scikit-build should find the venv numpy before system numpy.

Sorry, something went wrong.

Uh oh!

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

bnavigator reacted with thumbs up emoji

All reactions

👍 1 reaction

bnavigator Dec 30, 2020

Copy link

Copy Markdown

Collaborator 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.

This recipe is taken from the conda-forge feedstock.

Sorry, something went wrong.

Uh oh!

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

All reactions

bnavigator Dec 31, 2020

Copy link

Copy Markdown

Collaborator 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.

Are we sure the problem is not the fiddling with the python path in

Slycot/CMakeLists.txt

Lines 21 to 32 in 055e9d6

	# base site dir, use python installation for location specific includes
	execute_process(
	COMMAND "${PYTHON_EXECUTABLE}" -c
	"import os,numpy; print(os.path.dirname(numpy.__path__[0]))"
	OUTPUT_VARIABLE PYTHON_SITE
	OUTPUT_STRIP_TRAILING_WHITESPACE)
	if(WIN32)
	string(REPLACE "\\" "/" PYTHON_SITE ${PYTHON_SITE})
	endif()

	find_package(PythonLibs REQUIRED)
	find_package(NumPy REQUIRED)

find_package(NumPy ) is not a scikit-build feature but from CMake.

Sorry, something went wrong.

Uh oh!

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

All reactions

bnavigator Dec 31, 2020

Copy link

Copy Markdown

Collaborator 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.

find_package(NumPy ) is not a scikit-build feature but from CMake.

I am wrong here. Scikit-build's FindNumpy is a different thing than FindPython with NumPy component from CMake.

Sorry, something went wrong.

Uh oh!

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

All reactions

roryyorke Jan 1, 2021

Copy link

Copy Markdown

Collaborator

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.

Cool - that find_package finds the right thing without extra magic. I've noted that on the scikit-build issue.

Sorry, something went wrong.

Uh oh!

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

All reactions

bnavigator Jan 1, 2021

Copy link

Copy Markdown

Collaborator 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.

It might not help us (see my comment linking to the disussion there), because it sets different output variables in cmake. We need to find the correct f2py too. I was experimenting with FindF2PY from scikit-build yesterday. Curious if all the old windows hacks in setup.py and the CMake files are still necessary. They do not seem to work for my setup in the workflow so far.

Sorry, something went wrong.

Uh oh!

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

All reactions

roryyorke commented Dec 30, 2020

Copy link

Copy Markdown

Collaborator

Thanks for working on this!

It looks like this PR will drop Python <= 3.5 (I think Numpy is about to go >= 3.7 anyway); I think that's worth explicitly noting.

All reactions

Sorry, something went wrong.

Uh oh!

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

bnavigator commented Dec 30, 2020

Copy link

Copy Markdown

Collaborator Author

It looks like this PR will drop Python <= 3.5 (I think Numpy is about to go >= 3.7 anyway); I think that's worth explicitly noting.

All reactions

Sorry, something went wrong.

Uh oh!

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

bnavigator commented Dec 30, 2020

Copy link

Copy Markdown

Collaborator Author

Streamline conda recipes / sync with conda-forge feedstock?

Streamline LAPACK linking? The conda-forge packages nowadays always link against the reference implementation but you can use a different one (=the same as numpy) during runtime.

@repagh, I remember that prior to the 0.4.0 release you advocated for keeping these different conda recipes. Is this still true? Do we need to test them all?

All reactions

Sorry, something went wrong.

Uh oh!

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

bnavigator force-pushed the github-workflow branch 3 times, most recently from 12aefb6 to 65ff690 Compare

December 31, 2020 14:34

bnavigator linked an issue

Dec 31, 2020

that may be closed by this pull request

Add CI test for import slycot; slycot.test #137

Closed

bnavigator force-pushed the github-workflow branch 8 times, most recently from 4d9f1d8 to 44691f0 Compare

January 7, 2021 19:44

This was referenced Jan 7, 2021

use __call__ instead of evalfr in lti system classes python-control/python-control#449

Merged

Clean build setup #142

Merged

bnavigator force-pushed the github-workflow branch 4 times, most recently from 3325730 to 8f1c90b Compare

January 8, 2021 14:40

roryyorke mentioned this pull request

Jan 9, 2021

Merge conda recipes #143

Merged

bnavigator force-pushed the github-workflow branch 2 times, most recently from 870ee8c to c456fdd Compare

January 10, 2021 18:36

bnavigator mentioned this pull request

Jan 10, 2021

CI tests via GitHub actions and conda python-control/python-control#504

Merged

bnavigator force-pushed the github-workflow branch 2 times, most recently from b7801b6 to 05a0ad9 Compare

January 10, 2021 22:33

bnavigator force-pushed the github-workflow branch 3 times, most recently from 56de0c9 to 9abde05 Compare

January 11, 2021 20:06


          merge conda-recipes

77b28a4

bnavigator force-pushed the github-workflow branch 2 times, most recently from 6c47df1 to 8c216a3 Compare

January 11, 2021 22:06

bnavigator marked this pull request as ready for review

January 11, 2021 23:03

bnavigator mentioned this pull request

Jan 11, 2021

build & test using GitHub Workflows #80

Closed

bnavigator force-pushed the github-workflow branch from 8c216a3 to f63397c Compare

January 14, 2021 00:05

roryyorke commented Jan 24, 2021

Copy link

Copy Markdown

Collaborator

This looks good, but I'm in no position to review the details. One thing I did wonder about: why are the test matrices generated by scripts, and not known upfront?

Will we remove Travis CI config after this is merged?

For anyone else reviewing, you can see output here: https://github.com/bnavigator/Slycot/actions

The warnings there seem to be due to a third-party package; @bnavigator has already queried it conda-incubator/setup-miniconda#132 (comment)

Given that free Travis CI is disappearing, I think we should merge this. @murrayrm , @repagh, objections?

All reactions

Sorry, something went wrong.

Uh oh!

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

bnavigator commented Jan 24, 2021

Copy link

Copy Markdown

Collaborator Author

why are the test matrices generated by scripts, and not known upfront?

The test matrix generation is based upon which wheel and conda packages are created. With all the exceptions and special cases for combinations of OS and LAPACK implementation, statically defining the matrices would be very complicated.

All reactions

Sorry, something went wrong.

Uh oh!

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

bnavigator added 6 commits

January 24, 2021 10:25


          Update REAMDE.rst for conda single recipe and general compiling instr…

f5c5db4

…uctions


          update MANIFEST

2e22674


          Github actions workflow with split jobs


          Update README.rst and MANIFEST.in

d1c056a


          add fast sniff build

2a6a98f


          fix coveralls endpoint service

c58c564

bnavigator force-pushed the github-workflow branch from 898421a to c58c564 Compare

January 24, 2021 09:27

murrayrm commented Jan 24, 2021

Copy link

Copy Markdown

Member

No objections to merging on my end. I note that the full CI build seems to take ~30 minutes, but of course it is testing on all sorts of different configurations and OS's.

All reactions

Sorry, something went wrong.

Uh oh!

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

bnavigator commented Jan 24, 2021

Copy link

Copy Markdown

Collaborator Author

Most of the time is wasted by the conda resolver. Especially on Windows. This could be fixed be a frozen environment, which we update manually, but that is maybe something for another PR.

All reactions

Sorry, something went wrong.

Uh oh!

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

roryyorke merged commit a070bbb into python-control:master

Jan 30, 2021

bnavigator deleted the github-workflow branch

May 3, 2022 19:07

Reviewers

roryyorke

roryyorke 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

No milestone

Development

Successfully merging this pull request may close these issues.

Add CI test for import slycot; slycot.test Consider switching CI to Github Workflows, include Windows

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.

GeezerVison

Conversation

bnavigator commented Dec 30, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Builds on #143

Uh oh!

roryyorke Dec 30, 2020

Choose a reason for hiding this comment

Uh oh!

bnavigator Dec 30, 2020

Choose a reason for hiding this comment

Uh oh!

bnavigator Dec 31, 2020

Choose a reason for hiding this comment

Uh oh!

bnavigator Dec 31, 2020

Choose a reason for hiding this comment

Uh oh!

roryyorke Jan 1, 2021

Choose a reason for hiding this comment

Uh oh!

bnavigator Jan 1, 2021

Choose a reason for hiding this comment

Uh oh!

roryyorke commented Dec 30, 2020

Uh oh!

bnavigator commented Dec 30, 2020

Uh oh!

bnavigator commented Dec 30, 2020

Uh oh!

roryyorke commented Jan 24, 2021

Uh oh!

bnavigator commented Jan 24, 2021

Uh oh!

murrayrm commented Jan 24, 2021

Uh oh!

bnavigator commented Jan 24, 2021

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

bnavigator commented Dec 30, 2020 •

edited

Loading

Uh oh!

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