GitHub workflow

[bnavigator]

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

[roryyorke]

I just filed scikit-build/scikit-build#524 because I think scikit-build should find the venv numpy before system numpy.

[bnavigator]

This recipe is taken from the conda-forge feedstock.

[bnavigator]

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.

[bnavigator]

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.

[roryyorke]

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

[bnavigator]

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.

[roryyorke]

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.

[bnavigator]

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.

See also python-control/python-control#481

[bnavigator]

• 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?

[roryyorke]

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?

[bnavigator]

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.

[murrayrm]

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.

[bnavigator]

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.