Move to `pyproject.toml` build #218

LecrisUT · 2023-01-15T16:35:45Z

This PR has a few changes:

Rename and switch the ctest CI to be purely cmake. Didn't make much sense to be python dependent there
Switch the build to use pyproject.toml environment. Some equivalent commands to the setup.py ones are:
- setup.py build -> python -m build (need to install pip install build). This is mostly unnecessary unless you want to fail quickly
- setup.py install -> pip install .
Temporarily removed the nanoversion script until we think of how to implement it with dynamic variable Extract metadata from cmake scikit-build/scikit-build-core#172

We need to do a few tests on this:

Test the build CI. Setting the target branch to develop initially to trigger these (CI job finished)
Mac build passes with scikit-build-core
Test pypi-weel-builds. We'll change the target branch to rc if develop works to test it there (and if necessary back to develop to keep the branches synced)

@atztogo please let me know how the mac build goes. You can manually set it -DCMAKE_OSX_DEPLOYMENT_TARGET=... in pyprojec.toml if it doesn't work automatically and I'll write a PR upstream for that fix.

About making editable installs, this should be doable via pip install --editable .. But for this we need scikit-build/scikit-build-core#171

Closes #217
Currently, cibuildwheel still doesn't work locally. Added upstream issue: scikit-build/scikit-build-core#173. Here's the setup to debug locally assuming you have a docker/podman environment:

$ CIBW_CONTAINER_ENGINE=podman cibuildwheel . --platform linux

codecov-commenter · 2023-01-15T16:42:24Z

Codecov Report

Base: 64.00% // Head: 93.61% // Increases project coverage by +29.60% 🎉

Coverage data is based on head (07f0867) compared to base (6b4332c).
Patch has no changes to coverable lines.

Additional details and impacted files

@@             Coverage Diff              @@
##           develop     #218       +/-   ##
============================================
+ Coverage    64.00%   93.61%   +29.60%     
============================================
  Files           18       15        -3     
  Lines         1328      908      -420     
============================================
  Hits           850      850               
+ Misses         478       58      -420

Impacted Files	Coverage Δ
test/vasp.py
test/test_magnetic_dataset.py
test/test_niggli_delaunay.py
test/test_reciprocal_mesh.py
test/test_collinear_spin.py
test/conftest.py
test/test_benchmark.py
spglib/__init__.py
test/test_layergroup.py
test/test_spglib.py
... and 23 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

Signed-off-by: Cristian Le <cristian.le@mpsd.mpg.de>

LecrisUT · 2023-01-15T20:23:07Z

Worth pointing out. I have tested moving back to the old approach with setup.py, but we get again cmake not detecting python in cibuildwheel. There is something more fundamentally different here...

Edit: Oh wow, "the fix" is so stupid...

Signed-off-by: Cristian Le <cristian.le@mpsd.mpg.de>

LecrisUT · 2023-01-16T12:55:13Z

Sorry about the amount of CI actions. Cmake linking on windows is soo weird

Signed-off-by: Cristian Le <cristian.le@mpsd.mpg.de>

LecrisUT · 2023-01-16T16:03:28Z

@atztogo Finally got it working. Weirdly, the ubuntu tests take half an hour to perform. Any idea on that?

Edit: That was because it was running emulated aarch64 on x86_64

Good luck! Signed-off-by: Cristian Le <cristian.le@mpsd.mpg.de>

Signed-off-by: Cristian Le <cristian.le@mpsd.mpg.de>

atztogo · 2023-01-19T07:29:16Z

@LecrisUT, thanks for your great effort!

@atztogo please let me know how the mac build goes. You can manually set it -DCMAKE_OSX_DEPLOYMENT_TARGET=... in pyprojec.toml if it doesn't work automatically and I'll write a PR upstream for that fix.

I don't see what you mean. What can I do?

LecrisUT · 2023-01-19T08:11:24Z

Like what we did in #212. The CIs pass so I suspect it works, but confirm that please

lan496

@LecrisUT Great works, thanks! I have learned a lot about CMake and Python packaging from your PR. I wrote tiny comments.

pyproject.toml

CMakeLists.txt

Signed-off-by: Cristian Le <cristian.le@mpsd.mpg.de>

LecrisUT · 2023-01-19T10:18:56Z

Note that last commit is a [skip ci] so use merge/squash instead of rebase to trigger the github action again with it uploading to Pypi as well.

atztogo · 2023-01-19T12:56:30Z

Like what we did in #212. The CIs pass so I suspect it works, but confirm that please

Yes, on macOS arm64, I confirmed that it works. A warning was printed out in the build process by

pip install . -vvv

which is

[ 98%] Building C object python/CMakeFiles/python_api.dir/_spglib.c.o
  /Users/togo/code/spglib/python/_spglib.c:179:1: warning: unknown attribute 'dllexport' ignored [-Wunknown-attributes]
  DLL_PUBLIC PyObject* PyInit__spglib(void)
  ^~~~~~~~~~
  /Users/togo/code/spglib/python/_spglib.c:177:36: note: expanded from macro 'DLL_PUBLIC'
  #define DLL_PUBLIC __attribute__ ((dllexport))
                                     ^~~~~~~~~
  1 warning generated.

LecrisUT · 2023-01-19T13:26:47Z

Great, this should be ready now. I've cleaned the warning a bit for more general systems.

Also I've removed the Python2 compatibility macros since it has been sunset-ed for 3 years and we do not test them. Build system would not work for it also.

Signed-off-by: Cristian Le <cristian.le@mpsd.mpg.de>

atztogo · 2023-01-20T00:03:21Z

Merged to rc. Thanks a lot @LecrisUT!

LecrisUT marked this pull request as draft January 15, 2023 16:35

LecrisUT changed the base branch from develop to rc January 15, 2023 18:51

LecrisUT added 4 commits January 15, 2023 20:08

Switch to using only cmake for ctest

a6175fd

Signed-off-by: Cristian Le <cristian.le@mpsd.mpg.de>

Move python build commands to pyproject.toml with scikit-build-core

b32475a

Signed-off-by: Cristian Le <cristian.le@mpsd.mpg.de>

Minor CI updates

9363d5b

Signed-off-by: Cristian Le <cristian.le@mpsd.mpg.de>

Run pypi-wheel-builds on PRs as well

34822e1

Signed-off-by: Cristian Le <cristian.le@mpsd.mpg.de>

LecrisUT force-pushed the fix/217 branch from 88d9149 to 34822e1 Compare January 15, 2023 19:08

Ridiculous "fix" for cibuildwheel

51573de

Signed-off-by: Cristian Le <cristian.le@mpsd.mpg.de>

LecrisUT force-pushed the fix/217 branch 2 times, most recently from 8cf0302 to 18a122c Compare January 16, 2023 10:57

LecrisUT force-pushed the fix/217 branch 4 times, most recently from c4064b7 to ddef3e4 Compare January 16, 2023 14:18

Fix Windows build python api

aad8cca

Signed-off-by: Cristian Le <cristian.le@mpsd.mpg.de>

LecrisUT force-pushed the fix/217 branch from ddef3e4 to aad8cca Compare January 16, 2023 14:23

Disable pytest on cibuildwheel for emulated architectures

ed3d780

Good luck! Signed-off-by: Cristian Le <cristian.le@mpsd.mpg.de>

LecrisUT force-pushed the fix/217 branch from 6573d9d to ed3d780 Compare January 16, 2023 17:27

LecrisUT marked this pull request as ready for review January 16, 2023 17:29

Bump version 2.1.0

d95793f

Signed-off-by: Cristian Le <cristian.le@mpsd.mpg.de>

lan496 reviewed Jan 19, 2023

View reviewed changes

pyproject.toml Show resolved Hide resolved

CMakeLists.txt Outdated Show resolved Hide resolved

[skip ci] Update documentation and cmake requirements

6954fcc

Signed-off-by: Cristian Le <cristian.le@mpsd.mpg.de>

LecrisUT added 2 commits January 19, 2023 14:35

Generalize pyton export macro for non-windows

fd1f7ad

Signed-off-by: Cristian Le <cristian.le@mpsd.mpg.de>

Remove Python2 compatibility macros

3ea3e40

Signed-off-by: Cristian Le <cristian.le@mpsd.mpg.de>

LecrisUT force-pushed the fix/217 branch from 61d0349 to 3ea3e40 Compare January 19, 2023 13:36

lan496 approved these changes Jan 19, 2023

View reviewed changes

atztogo merged commit 9c5b02c into spglib:rc Jan 20, 2023

lan496 mentioned this pull request Jan 20, 2023

Sync pyproject.toml build in rc branch to develop #219

Merged

LecrisUT deleted the fix/217 branch January 20, 2023 09:36

atztogo mentioned this pull request Jan 21, 2023

PyPI build failure on github workflow #217

Closed

LecrisUT added this to the 2.1 milestone Feb 21, 2023

LecrisUT added the enhancement Significant ehancements label Mar 1, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Move to `pyproject.toml` build #218

Move to `pyproject.toml` build #218

Uh oh!

LecrisUT commented Jan 15, 2023 •

edited

Loading

Uh oh!

codecov-commenter commented Jan 15, 2023 •

edited

Loading

Uh oh!

LecrisUT commented Jan 15, 2023 •

edited

Loading

Uh oh!

LecrisUT commented Jan 16, 2023

Uh oh!

LecrisUT commented Jan 16, 2023 •

edited

Loading

Uh oh!

atztogo commented Jan 19, 2023

Uh oh!

LecrisUT commented Jan 19, 2023

Uh oh!

lan496 left a comment

Uh oh!

Uh oh!

Uh oh!

LecrisUT commented Jan 19, 2023

Uh oh!

atztogo commented Jan 19, 2023

Uh oh!

LecrisUT commented Jan 19, 2023

Uh oh!

atztogo commented Jan 20, 2023

Uh oh!

Uh oh!

Move to pyproject.toml build #218

Move to pyproject.toml build #218

Uh oh!

Conversation

LecrisUT commented Jan 15, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov-commenter commented Jan 15, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

LecrisUT commented Jan 15, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

LecrisUT commented Jan 16, 2023

Uh oh!

LecrisUT commented Jan 16, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

atztogo commented Jan 19, 2023

Uh oh!

LecrisUT commented Jan 19, 2023

Uh oh!

lan496 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

LecrisUT commented Jan 19, 2023

Uh oh!

atztogo commented Jan 19, 2023

Uh oh!

LecrisUT commented Jan 19, 2023

Uh oh!

atztogo commented Jan 20, 2023

Uh oh!

Uh oh!

Move to `pyproject.toml` build #218

Move to `pyproject.toml` build #218

LecrisUT commented Jan 15, 2023 •

edited

Loading

codecov-commenter commented Jan 15, 2023 •

edited

Loading

LecrisUT commented Jan 15, 2023 •

edited

Loading

LecrisUT commented Jan 16, 2023 •

edited

Loading