flare icon indicating copy to clipboard operation
flare copied to clipboard

chore: move to scikit-build-core

Open henryiii opened this issue 2 years ago • 1 comments

Summary

Include a summary of major changes in bullet points:

  • Use scikit-build-core
  • Set version in pyproject.toml

Additional dependencies introduced (if any)

  • Scikit-build-core (removing some too, including setuptools)

TODO (if any)

  • Test pip install

Checklist

Work-in-progress pull requests are encouraged, but please put [WIP] in the pull request title.

Before a pull request can be merged, the following items must be checked:

  • [ ] Code is in the standard Python style. Run Black on your local machine.
  • [ ] Docstrings have been added in the Sphinx docstring format.
  • [ ] Type annotations are highly encouraged.
  • [ ] Tests have been added for any new functionality or bug fixes.
  • [ ] All existing tests pass.
  • [ ] The version number in flare/_version.py is updated. We are using a version number format a.b.c
    • If this PR fixes bugs, update version number to a.b.c+1
    • If this PR adds new features, update version number to a.b+1.0
    • If this PR includes significant changes in framework or interface, update version number to a+1.0.0

Note that the CI system will run all the above checks. But it will be much more efficient if you already fix most errors prior to submitting the PR.

henryiii avatar Jul 15 '23 16:07 henryiii

Adding Python_SITELIB to CMAKE_PREFIX_PATH would make vanilla CMake runs work like scikit-build-core and find pip/conda installed pybind11 by default.

henryiii avatar Jul 15 '23 20:07 henryiii

@anjohan, is this something we want to pursue? I'm inclined to close given the age of these commits and the failing builds

jonpvandermause avatar Sep 16 '24 02:09 jonpvandermause

Let me know if you want me to rebase.

henryiii avatar Sep 16 '24 03:09 henryiii

Thanks @henryiii, that would be helpful. As long as we can get the build passing I don't see a reason not to use scikit-build-core; it looks like it would be a solid improvement over our current approach.

jonpvandermause avatar Sep 16 '24 03:09 jonpvandermause

@anjohan, is this something we want to pursue? I'm inclined to close given the age of these commits and the failing builds

Yes, ideally, as I think this would be a substantial improvement (and simplification). It's just been below the greater cleanup (deleting 2+3, old OTF etc.) on the priority list.

anjohan avatar Sep 16 '24 16:09 anjohan

Yes, ideally, as I think this would be a substantial improvement (and simplification).

Yeah, I've looked into it a bit and agree, definitely an improvement. @henryiii I'm rebasing on a separate branch, so no need to spend time on this -- I will let you know if I hit any snags.

It's just been below the greater cleanup (deleting 2+3, old OTF etc.) on the priority list.

We should probably talk about the plan here, I would prefer to keep the old code in place in case other groups want to reproduce published results or try similar experiments

jonpvandermause avatar Sep 16 '24 16:09 jonpvandermause

Looks like github is showing the entire history of the repo since @henryiii's fork was created, making the merge pretty hard to read. I've made an identical but cleaner version of this PR over at #416, so I'm going to close this one out.

jonpvandermause avatar Sep 17 '24 02:09 jonpvandermause