Skip to content
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

change build system to use Cmake #9

Merged
merged 11 commits into from
Apr 25, 2023
Merged

change build system to use Cmake #9

merged 11 commits into from
Apr 25, 2023

Conversation

cwsimmon
Copy link
Collaborator

Updated build environment to use CMake and integrated with pybind11 and pybmds. CMake builds should work for Windows, Mac, and Linux.

Copy link
Collaborator

@shapiromatron shapiromatron left a comment

Choose a reason for hiding this comment

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

Looking good, a few minor tweaks. I'll see if I can fix the test in CI.

src/include/old/automobile Outdated Show resolved Hide resolved
src/pybmdscpp/ex3.cpp Outdated Show resolved Hide resolved
src/pybmdscpp/main.cpp Outdated Show resolved Hide resolved
tests/test_pybmds/test_RBMDS.py Outdated Show resolved Hide resolved
Copy link
Collaborator Author

@cwsimmon cwsimmon left a comment

Choose a reason for hiding this comment

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

Updated accorded to requested changes

Copy link
Collaborator

@shapiromatron shapiromatron left a comment

Choose a reason for hiding this comment

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

LGTM!

@@ -1,4 +1,4 @@
from pybmds import bmdscore, __version__
from pybmds import __version__, bmdscore
Copy link
Collaborator

@shapiromatron shapiromatron Apr 25, 2023

Choose a reason for hiding this comment

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

we have code enforcement to standardize how code is written so it's consistent. This error was caused because the order of the imports wasn't in abc order. To fix, I ran make format and the formatter fixed it for me. You can google black, flake8, and isort for more info on our linters, though we're about to switch to the uber linter ruff that does all those and more.

@@ -27,7 +27,8 @@
const double BMDS_EPS = 1.0e-6;
const double BMDS_MISSING = -9999.0;
const double BMDS_QNORM = 1.959964; //bound for 95% confidence interval
const char BMDS_VERSION[32] = "2023.03.1";
//const char BMDS_VERSION[32] = "2023.03.1";
Copy link
Collaborator

Choose a reason for hiding this comment

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

remove these comments?

@shapiromatron shapiromatron changed the title Cmake change build system to use Cmake Apr 25, 2023
@cwsimmon cwsimmon merged commit 9bfd95f into main Apr 25, 2023
@cwsimmon cwsimmon deleted the cmake branch April 25, 2023 20:53
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