-
Notifications
You must be signed in to change notification settings - Fork 15
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
refactor(build): change setup.py to pyproject.toml and supress cython warning #293
Conversation
@@ -864,23 +865,6 @@ def omas_global_quantities(imas_version=omas_rcparams['default_imas_version']): | |||
return _global_quantities[imas_version] | |||
|
|||
|
|||
# only attempt cython if effective user owns this copy of omas | |||
# disabled for Windows: need to add check for file ownership under Windows | |||
if os.name == 'nt' or os.geteuid() != os.stat(__file__).st_uid: |
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.
With this change the compiled version of omas_cython
should be available to everyone. Also windows users.
exclude = ["sphinx*"] | ||
|
||
[tool.setuptools.dynamic] | ||
version = {file = "omas/version"} |
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 automatically sets the version to what is found in omas/version
pyproject.toml
Outdated
] | ||
keywords = ["integrated modeling", "OMFIT", "IMAS", "ITER"] | ||
|
||
requires-python = '>=3.8' |
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 would require bumping this from 3.6 to 3.8.
3.6 was end of life in 2021, so I hope that this is ok?
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.
OMFIT is at python=3.7 and uses omas as an integral part. What changes are you making here that require python>=3.8?
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.
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.
@smithsp I double checked and I was correct that in theory 3.7
should have been sufficient to enable usage of pyproject.toml
based package configurations.
However, the problem here is the dependency on pygacode
which is forced to version 0.71
from 2022 because that's the last version that supports 3.7
, and that version does not correctly specify its build dependencies (requires numpy). This is fixed for versions >1.0
but those require python >=3.8
which is why the build here works for 3.8
but nor for 3.7
.
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.
We are actively working on migrating OMFIT to a later python version.
Stale pull request message |
I was motivated to fix #289, but when starting to work on the old
setup.py
I was curious if you would be open to a bit of a modernisation, see below:Changes in this PR:
pyproject.toml
following the adopted python standards.setup.py
that translatesomas_cython.pyx
toomas_cython.c
at the time the sdist package is created. This has the benefit thatCython
is no longer a dependency for users. They*.c
file still gets compiled during package install on the users computer, but that still is pretty quick and doesn't produce any warnings.This PR also removes the need to manually traverse the file hierarchy to create
packages
andpackage_data
lists. This is now automatically handled.Editable installs still work by simply running:
To build a sdist package using
build
run:from the root of the project. This creates an isolated virtual environment to build the sdist according to the spec in
pyproject.toml
.