-
Notifications
You must be signed in to change notification settings - Fork 13
Cmake version and CI python version upgrades #178
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
Merged
Merged
Changes from 9 commits
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
fe3609a
cmake version and CI python version upgrades
varunagrawal 596f592
use latest runner versions
varunagrawal 3eeb497
use uv for project management
varunagrawal 7ede056
improve CI thanks to uv
varunagrawal 5ce2c07
minor updates
varunagrawal f64d945
set min python version to 3.10
varunagrawal 36c266f
update lock file
varunagrawal bf6684f
remove even more unneeded dependencies
varunagrawal 74a4428
add additional authors
varunagrawal 9f82c7e
make minimum cmake version 3.19.2 since it supports Apple Silicon
varunagrawal 4f169c7
Set minimum cmake version to be 3.22 to support Ubuntu 22.04
varunagrawal File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,3 +11,5 @@ __pycache__/ | |
| **/.coverage | ||
|
|
||
| gtwrap/matlab_wrapper/matlab_wrapper.tpl | ||
|
|
||
| .python-version | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,52 @@ | ||
| [project] | ||
| name = "gtwrap" | ||
| version = "2.0.0" | ||
| description = "Library to wrap C++ with Python and Matlab" | ||
| authors = [ | ||
| { name="Frank Dellaert", email="dellaert@gatech.edu" }, | ||
| { name = "Varun Agrawal", email = "varunagrawal@gatech.edu" }, | ||
| { name = "Duy Nguyen Ta", email = "duynguyen@gatech.edu" }, | ||
| { name = "Fan Jiang", email = "i@fanjiang.me" }, | ||
| { name = "Matthew Sklar", email = "matthewsklar227@gmail.com" } | ||
| ] | ||
| readme = "README.md" | ||
| license = "BSD-3-Clause" | ||
| keywords=["wrap", "bindings", "cpp", "python", "matlab"] | ||
|
|
||
| classifiers = [ | ||
| "Intended Audience :: Education", | ||
| "Intended Audience :: Developers", | ||
| "Intended Audience :: Science/Research", | ||
| "Operating System :: MacOS", | ||
| "Operating System :: Microsoft :: Windows", | ||
| "Operating System :: POSIX :: Linux", | ||
| "Programming Language :: Python", | ||
| "Programming Language :: Python :: 3.11", | ||
| ] | ||
|
|
||
| requires-python = ">=3.10" | ||
| dependencies = [ | ||
| "pyparsing>=3.2.5", | ||
| ] | ||
|
|
||
| [dependency-groups] | ||
| dev = [ | ||
| "pytest>=9.0.1", | ||
| "pytest-cov>=7.0.0", | ||
| ] | ||
|
|
||
| [project.urls] | ||
| Homepage = "https://github.com/borglab/wrap" | ||
| Documentation = "https://github.com/borglab/wrap" | ||
| Repository = "https://github.com/borglab/wrap" | ||
|
|
||
| [build-system] | ||
| requires = ["hatchling"] | ||
| build-backend = "hatchling.build" | ||
|
|
||
| [tool.pytest.ini_options] | ||
| addopts = "--cov=gtwrap --cov-report html --cov-report term" | ||
| norecursedirs = [] | ||
|
|
||
| [tool.uv.build-backend] | ||
| module-name = "gtwrap" |
This file was deleted.
Oops, something went wrong.
This file was deleted.
Oops, something went wrong.
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
3.30 is really really new
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.
Cmake is currently on version 4. Plus I figured this is more for our internal use anyway.
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.
Could we use whatever we currently use in GTSAM? Same goes for CI runners, btw. Otherwise we could get in a situation where we use something here that is not yet supported in GTSAM.
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.
Yeah, CMake will default to use the cmake config provided by upstream package even with
FindBoost, see https://cmake.org/cmake/help/v3.10/module/FindBoost.html#boost-cmakeSo we don't need to bump the minimum requirement, just need to have a test for the existence of the CMP rule and set it to NEW if exists to silence the warning
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.
I've made that change in GTSAM and was going to push it along with some other minor fixes.
(CMake had been yelling at me a lot recently)
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.
Main project is at 3.9 right now https://github.com/borglab/gtsam/blob/develop/CMakeLists.txt
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.
Asking for too new a version imposes a burden on our users. What is the default version installed for Ubuntu 22 and 24? What is the version in brew and whatever windows uses (Unix subsystem uses Ubuntu). That is what should drive this.
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.
I made that change locally, I haven't pushed it yet since I am trying to verify dependencies.
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.
I don't have the bandwidth to deal with this (IMO trivial) issue, so I dropped the version down to 3.19.2 since that version is when CMake started supporting Apple Silicon.
(I say trivial since I already spent the time compiling and testing compatibility already before making the PR, as a good contributor would)