Skip to content

Conversation

@msimberg
Copy link
Contributor

@msimberg msimberg commented Feb 24, 2025

This removes the cpp_bindgen tests and examples from gridtools.

If cpp_bindgen will be used with gridtools storages, we should recover the tests, possibly in the cpp_bindgen repo.

Comment on lines 1 to 2
TODO: Remove? Or are the instructions still valid for using cpp_bindgen separately?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Whether or not this file should be removed is I think my main question. Some of it still applies since the text talks about fetch-contenting cpp_bindgen. However, the headers from gridtools that use cpp_bindgen are removed with this PR, so that part is definitely not going to work.

Do the nanobind headers completely replace all the cpp_bindgen functionality, or not fully?

Copy link
Contributor

Choose a reason for hiding this comment

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

Good question. Not sure what's the best option. Maybe we leave this documentation here and also keep the headers for backwards-compatibility.

nanobind just does Python <-> C++ bindings, cpp_bindgen just does Fortran <-> C++ bindings. So for GT4Py use-case cpp_bindgen is not relevant, but in case someone has a Fortran <-> C++ use-case, I would go with cpp_bindgen.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good to me. Would you keep the fortran_array_view.hpp and fortran_array_adapter.hpp headers where they are, but without any guarantees (they'll likely bitrot if all the tests that use them are removed; but it might not matter if gridtools isn't going to have many changes anymore)?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, not perfect. I would move the testing to cpp_bindgen (if there would be a use-case)...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Restored the headers in e387ed4.

@msimberg
Copy link
Contributor Author

I see there's a full docs directory checked in with built documentation. Should that stay and if so, should it be updated?

@msimberg
Copy link
Contributor Author

cscs-ci run

Copy link
Contributor

@havogt havogt left a comment

Choose a reason for hiding this comment

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

Thanks!

@havogt havogt merged commit 7bb4710 into GridTools:master Feb 26, 2025
69 checks passed
havogt pushed a commit that referenced this pull request Apr 11, 2025
This removes the `cpp_bindgen` tests and examples from gridtools.

If `cpp_bindgen` will be used with gridtools storages, we should recover
the tests, possibly in the `cpp_bindgen` repo.
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