-
Notifications
You must be signed in to change notification settings - Fork 8
Add TUV-x Grid/GridMap Python classes #606
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
Conversation
📄 Documentation for this branch is available at: https://ncar.github.io/musica/branch/develop-597-grid/ |
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.
Pull Request Overview
This PR adds Python classes for TUV-x grids and grid maps with dictionary-style syntax, allowing users to create, manage, and manipulate grids in Python. It includes enhanced error handling with meaningful error messages and exposes additional grid functionality like removal operations and index-based access.
- Added Grid and GridMap Python classes with dictionary-style access patterns
- Enhanced error handling with meaningful error codes and messages instead of generic codes
- Added new grid map operations: removal by name/index, get by index, and grid count functionality
Reviewed Changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 7 comments.
Show a summary per file
File | Description |
---|---|
src/tuvx/interface_grid_map.F90 | Added error constants, new grid operations (get by index, remove, count), and improved error handling |
src/tuvx/interface_grid.F90 | Added error constants, grid name/units getters, and improved error handling |
src/tuvx/grid_map.cpp | Enhanced error handling with meaningful messages and added new GridMap operations |
src/tuvx/grid.cpp | Enhanced error handling and added grid name/units getters |
src/test/unit/tuvx/tuvx_c_api.cpp | Updated tests to use new API functions and test new functionality |
musica/tuvx.py | Added GridMap import for Python interface |
musica/tuvx.cpp | Minor formatting fix |
musica/test/unit/test_grid_map.py | New comprehensive test suite for GridMap class |
musica/test/unit/test_grid.py | New comprehensive test suite for Grid class |
musica/grid_map.py | New Python wrapper providing dictionary-style GridMap interface |
musica/grid_map.cpp | New Python bindings for GridMap class |
musica/grid.py | New Python wrapper for Grid class with enhanced interface |
musica/grid.cpp | New Python bindings for Grid class |
musica/binding_common.cpp | Added grid and grid_map binding functions |
musica/CMakeLists.txt | Added grid.cpp and grid_map.cpp to build |
include/musica/tuvx/grid_map.hpp | Added new GridMap methods and function declarations |
include/musica/tuvx/grid.hpp | Added grid name/units getters and updated function names |
fortran/tuvx/grid.F90 | Updated function name for consistency |
cmake/dependencies.cmake | Updated TUV-x dependency to newer version |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #606 +/- ##
==========================================
- Coverage 80.81% 79.92% -0.89%
==========================================
Files 54 54
Lines 6040 6367 +327
==========================================
+ Hits 4881 5089 +208
- Misses 1159 1278 +119 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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.
Looks good to me!
py::buffer_info buf = array.request(); | ||
musica::Error error; | ||
size_t size = self.GetNumberOfSections(&error) + 1; | ||
if (!musica::IsSuccess(error)) |
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.
This is yet another way to pass arrays from python to c++, and is different from what we did in CARMA, but I think I like this better
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.
This was a Copilot suggestion, but I liked it too.
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 think I do like the monkey patch thing in the python classes. I'm hoping that we can replicate the same thing with the mechanism configuration objects
I liked it too! It was easy to implement and I feel like the code is easy to read. |
This PR adds the ability to create and use TUV-x grids and grid maps in Python. It makes use of the now public data members of the TUV-x
grid_t
class to directly modify the collection of grids in agrid_warehouse_t
so that errors can be properly raised and to fill in gaps of functionality of thegrid_warehouse_t
class (e.g., removing grids, finding grids by index).An attempt was made to have the
GridMap
class mimic the syntax of a Python dictionary in anticipation of porting TUV-x to C++/Python.The error handling for grids and grid maps in the C/C++ library was updated to return more meaningful error messages.
closes #597