Skip to content

Move scripts for building wheels to CI directory#2044

Merged
varunagrawal merged 2 commits intodevelopfrom
python-wheel
Mar 10, 2025
Merged

Move scripts for building wheels to CI directory#2044
varunagrawal merged 2 commits intodevelopfrom
python-wheel

Conversation

@varunagrawal
Copy link
Copy Markdown
Contributor

Since we use CMake for building GTSAM, most users will have a build directory, and this may cause confusion with the newly added build_tools directory.

This PR moves the cibuildwheel scripts to .github/scripts/python_wheels so that everything is nicely packaged under a single directory tree (which is .github).

@yambati03 you may have had some good design choices when creating build_tools, which I would really like to understand. 🙂

@varunagrawal varunagrawal requested a review from yambati03 March 7, 2025 21:10
@varunagrawal varunagrawal self-assigned this Mar 7, 2025
Copy link
Copy Markdown
Contributor

@yambati03 yambati03 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me! I originally had them in a top-level build_tools directory since it seemed like the common practice across other large open source projects such as scikit-learn and numpy. However, I can see this being confusing and moving them to the .github/scripts folder makes it clear that these are associated with the Github Actions workflow. Thanks!

@varunagrawal
Copy link
Copy Markdown
Contributor Author

Got it! That actually makes a lot of sense if we were a python oriented codebase. Thanks for the information. :)

@varunagrawal varunagrawal merged commit aa0eb11 into develop Mar 10, 2025
36 checks passed
@varunagrawal varunagrawal deleted the python-wheel branch March 10, 2025 00:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants