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

CMake: Build Python API #2298

Merged
merged 14 commits into from
Jun 11, 2021
Merged

CMake: Build Python API #2298

merged 14 commits into from
Jun 11, 2021

Conversation

dschwoerer
Copy link
Contributor

@dschwoerer dschwoerer commented May 1, 2021

Add cmake instructions for boutcore

Resolves #1940
Resolves #2289

I need to remove the commented out code, but maybe @ZedThree also has some further ideas to cleanup.

I am not sure about auto-enabling. The issue is that in order to build a .so, -fPIC is needed, which seems to be only inserted with BUILD_SHARED_LIBS=ON.

I also just noticed I need to move this a bit further down, so it is only enabled when we are building the interface.

if (NOT BUILD_SHARED_LIBS)

dschwoerer and others added 5 commits May 1, 2021 20:51
The files are part of $(TOGEN)
If requested, but requirements are not present, this will fail, rather
then ignore the requested option
Default is to enable, if requirements are present.
@ZedThree
Copy link
Member

ZedThree commented May 4, 2021

Awesome, thanks @dschwoerer !

Overall it looks good. To make it a bit cleaner, I'd probably move the numpy stuff into cmake/FindNumpy.cmake (similarly for Cython), and then maybe the bulk of the boutcore-specific stuff to tools/pylib/boutcore_/CMakeLists.txt (and then add_subdirectory(...))

I'm not sure about forcing shared libs on. It might be nicer to instead have an overall default of shared libs and then a check and fatal error if they're not enabled.

Copy link
Member

@ZedThree ZedThree left a comment

Choose a reason for hiding this comment

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

Looks good, a few pointers on how to polish the CMake a bit

cmake/FindNumpy.cmake Outdated Show resolved Hide resolved
cmake/FindCython.cmake Outdated Show resolved Hide resolved
cmake/FindNumpy.cmake Outdated Show resolved Hide resolved
tools/pylib/_boutcore_build/CMakeLists.txt Outdated Show resolved Hide resolved
tools/pylib/_boutcore_build/CMakeLists.txt Outdated Show resolved Hide resolved
tools/pylib/_boutcore_build/CMakeLists.txt Outdated Show resolved Hide resolved
tools/pylib/_boutcore_build/CMakeLists.txt Outdated Show resolved Hide resolved
tools/pylib/_boutcore_build/CMakeLists.txt Outdated Show resolved Hide resolved
tools/pylib/_boutcore_build/CMakeLists.txt Outdated Show resolved Hide resolved
tools/pylib/_boutcore_build/CMakeLists.txt Outdated Show resolved Hide resolved
* next: (349 commits)
  Delete integrated test of FieldFactory: replaced by unit tests
  Convert ersatz MMS tests to use library MMS features
  Disable tests by default if building as part of another project
  CMake rename PACKAGE_TESTS to BOUT_TESTS
  Add some docs for formatting `Options` to string
  Fix for conditionally used boundary condition inputs in 'all'
  Further improve test-invpar
  fix WithQuietOutput
  speedup test-invpar
  Update .gitignore
  Enable running tests in parallel
  Add test for invalid Options format string
  Fix more clang-tidy suggestions
  Fix clang-tidy suggestions
  Replace OptionINI::writeSection with fmt::format
  Print docstring and type info for unused inputs
  Fix `Options` formatting to handle values correctly
  Add more format specs for `Options`: key-only and source attribute
  Add `fmt::formatter` for `Options`
  Restrict Options implicit cast operator to only types in variant
  ...
ZedThree
ZedThree previously approved these changes Jun 11, 2021
@ZedThree
Copy link
Member

@dschwoerer I've made some changes, are you ok with them?

Now stops immediately if option was `MAYBE`, and FATAL_ERROR if option
was turned on
@dschwoerer
Copy link
Contributor Author

Yes, looks good. Sorry I didn't get around to it myself. Much appreciated 👍

@ZedThree ZedThree changed the title cmake: boutcore CMake: Build Python API Jun 11, 2021
@ZedThree ZedThree merged commit 2920aba into next Jun 11, 2021
@ZedThree ZedThree deleted the cmake-boutcore branch June 11, 2021 15:18
@ZedThree ZedThree mentioned this pull request Jul 2, 2021
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.

None yet

2 participants