GRIDEDIT-2301 Global grid always extends to poles - #285
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the MeshKernelPy API for generating a global 2D mesh so the latitude discretization is no longer caller-provided, aligning with the goal that the global grid extends to the poles by construction.
Changes:
- Changed
MeshKernel.mesh2d_make_globalto accept onlynum_longitude_nodesand removed the latitude-node argument. - Updated the corresponding basic tests to call the new one-argument API.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
meshkernel/meshkernel.py |
Updates the public wrapper method signature and its docstring for global grid generation. |
tests/test_mesh2d_basics.py |
Adjusts unit tests to use the new mesh2d_make_global(num_longitude_nodes) call pattern. |
Suppressed comments (1)
tests/test_mesh2d_basics.py:692
- Given the PR goal (“global grid always extends to poles”), this test only asserts array sizes, which won’t catch regressions where the latitude extent still doesn’t reach ±90°. Add explicit assertions on the latitude range (min/max of
node_y) to validate the new behavior.
mesh2d = mk.mesh2d_get()
assert mesh2d.edge_x.size == 1225
assert mesh2d.node_x.size == 621
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| def mesh2d_make_global(self, num_longitude_nodes: int) -> None: | ||
| """Compute the global mesh with a given number of points along the longitude direction. | ||
| The points in the latitude direction will continue to the poles. |
There was a problem hiding this comment.
This is the notebook referred to by Copilot: https://github.com/Deltares/MeshKernelPy/blob/main/docs/examples/10_mesh2d_global_grid.ipynb
90da444 to
70a9cbd
Compare
veenstrajelmer
left a comment
veenstrajelmer
left a comment
There was a problem hiding this comment.
One textual suggestion
| """Compute the global mesh with a given number of points along the longitude direction. | ||
| The points in the latitude direction will continue up to the poles. |
There was a problem hiding this comment.
I would recommend something like this, a bit more complete: "Compute the global mesh with a given number of points along the longitude direction. The number of points in the latitude direction will be automatically calculated, such that the resulting cells are as square as possible and the resulting mesh extends up to the poles."
Besides that: "points" is not terminology I am familiar with, what do you think of using "nodes" or "cells" or do you prefer the current "points"?
There was a problem hiding this comment.
I have one textual suggestion. And please also update the example notebook to not include the deprecated argument: https://github.com/Deltares/MeshKernelPy/blob/main/docs/examples/10_mesh2d_global_grid.ipynb
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
… call