Build update #796
Build update #796
Conversation
The original distutils-based system has been replaced with setuptools and pyproject.toml. C/C++ compiler version and flags are currently not checked; we should either re-implement the checks in setup.py or wait for PEP 725 (https://discuss.python.org/t/pep-725-specifying-external-dependencies-in-pyproject-toml/31888?page=1) to see the light of day.
This includes CI updates (adding optional modules) and renaming nosetests to tests.
|
As of right now, the C/C++ stuff looks like a show-stopper for my system. |
|
Can you please share your system details? Also, how are you trying to install it? pip with or without -e? |
|
Tried both with and without -e, still fails (albeit with a different error: using -e tells me to try not using -e). I'm on OS X 13.6.1 on a 2018 intel Mac I'm a bit confused about the error it throws (that could be because it's late and I'm tired though). Looks to me like it should be gcc-12 but then complains about a clang error - see attached |
|
The latest macos runner that github actions support is macos-12. Thus, I don't know how to test/troubleshoot macos-13 without having my hands on one. Does building sdist/wheel work? |
|
Nope, looks a lot like the same error. |
|
I think it might be your system, not phoebe. Can you link any c/c++ executables? |
|
A possible |
|
Yeah, this is weird. Can't link with g++-12 or g++13 but clang++ is working. So setting CC=clang worked. Problem was definitely on my end. |
@Raindogjones - is this something that you didn't need to do manually before though? And if it is just a weird setup, should we provide some advice somewhere on the install page for how to handle these cases? When we release this, I can also update the install page to suggest |
There was a problem hiding this comment.
Is all the renaming from "nosetests" to "tests" to actually migrate to pytest or is it required for the rest of the changes?
I am just worried about dealing with conflicts in feature branches as we try to propagate this there. If we don't need to do it now, maybe we add it to the list of changes to make once we can close out those feature branches?
|
@kecnry I don't think any of that is necessary. Seems this is a common problem of apple's xcode 15 not playing well with homebrew's gcc. I hadn't realised before because I only updated xcode ~a month ago and haven't done much coding since. Easy workarounds are to build gcc from source or to stick to clang. I also read somewhere that this is fixed in the current xcode beta so will go away soon. |
| do = ['tutorials', 'doctests', 'nosetests', 'pylint', 'benchmark'] | ||
| do = ['tutorials', 'doctests', 'tests', 'pylint', 'benchmark'] |
There was a problem hiding this comment.
this will also need a change to website instructions
It is not required for the build system switch, I just seized the opportunity since I was updating the CI action that runs the tests. Keeping it labeled as nosetests is confusing and might make people want to run nose instead of pytest. |
| 'phoebe.solverbackends.knn': ['*.knn'] | ||
| }, | ||
| ext_modules = ext_modules, | ||
| scripts=None if _env_variable_bool('PHOEBE_SKIP_SCRIPTS', False) else ['client-server/phoebe-server', 'client-server/phoebe-autofig'], |
There was a problem hiding this comment.
I missed this - but it seems this removed support for installing these scripts... can this be reintroduced in the current setup?
This is an upgrade from @Raindogjones' previous PR to replace distutils; this one goes farther by switching to pyproject.toml, using setuptools as backend and build/pip as frontend.
Note that C/C++ compiler versions/flags have not been ported (yet); the only currently supported way to do that is clunky (it goes back to setup.py with custom functions) but there's a PEP 725 under consideration that would streamline this significantly. @horvatm should weigh in on this whether we should re-add the tests into setup.py or await PEP 725.
Closes #644