-
Notifications
You must be signed in to change notification settings - Fork 29
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
move build configuration to pyproject.toml
#279
Conversation
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 @zacharyburnett! This is really helpful, sorry it took me some time to get around to it!
No problem! The CI seems to be failing because it never calls |
@zacharyburnett I noticed and am updating some of the docs/removing deprecated test runs etc. I am hoping to have it in better shape this afternoon! |
Are you sure this PR is complete?
Should not merge PR with red CI, if possible. |
"setuptools>=61.2", | ||
"setuptools_scm[toml]>=3.4", | ||
"wheel", | ||
"oldest-supported-numpy", |
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 you sure you need oldest-supported-numpy . Do you have C-extension that uses numpy ABI?
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.
@zacharyburnett I am assuming this a hold over from the setup.py
file. I can double check and open a new PR with a fix.
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.
Looks like we versioned numpy
in the setup.py
file. Any reason for using the "oldest-supported-numpy" 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 section is for build, not install. Do you need numpy during compile time?
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.
Ah, right sorry. I don't believe we do!
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.
Then you should remove it. Will make your lives easier when Python 3.12 and NumPy 2.0 are released.
I agree, but sadly the tests have been failing for some time now. There are some issues that are instrument specific that cause them which are out of @Witchblade101 and I's expertise. In this case I was wrong and should have double checked. |
@pllim maybe it was an issue with python 3.7 because @zacharyburnett just added another PR #319 and the test are all passing now 🎉 |
setuptools
now supports the[project]
table, which is defined by PEP621.Additionally,
setuptools
now supports its own entry inpyproject.toml
called[tool.setuptools]
(pypa/setuptools#1688, https://setuptools.pypa.io/en/latest/userguide/pyproject_config.html#setuptools-specific-configuration); however, it comes with the following disclaimer:Reading
toml
is supported natively in Python 3.11 withtomllib
Given this, we can attempt to consolidate the build configuration into a single
pyproject.toml
file that can possibly be read by other build systems in the future.