Skip to content

Conversation

@erikvansebille
Copy link
Member

@erikvansebille erikvansebille commented Aug 20, 2025

This PR cleans up the different ways in which mesh type information ("spherical" or "flat") was used in v4. It is now a property of the (U)XGrid class

Copy link
Contributor

@VeckoTheGecko VeckoTheGecko left a comment

Choose a reason for hiding this comment

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

Agreed that this does clean things up significantly.

Do we want to rename mesh to mesh_type? In v3 we used mesh, in v4-dev we were inconsistent (mesh and mesh_type). I agree we should just chose one, but I don't personally find mesh to be confusing naming (especially since we have type annotations), and the renaming of the public variable seems more confusing for users than keeping the original. This is a nitpick - if you feel strongly about mesh_type go for it.

@erikvansebille
Copy link
Member Author

Agreed that this does clean things up significantly.

Do we want to rename mesh to mesh_type? In v3 we used mesh, in v4-dev we were inconsistent (mesh and mesh_type). I agree we should just chose one, but I don't personally find mesh to be confusing naming (especially since we have type annotations), and the renaming of the public variable seems more confusing for users than keeping the original. This is a nitpick - if you feel strongly about mesh_type go for it.

I see your point about trying to keep the API as similar as possible. Let's indeed then go for mesh. I'll update the code

@erikvansebille erikvansebille changed the title Moving the mesh_type to the (U)XGrid class Moving the mesh information to the (U)XGrid class Aug 22, 2025
@erikvansebille erikvansebille merged commit 807d2ee into v4-dev Aug 22, 2025
8 of 9 checks passed
@erikvansebille erikvansebille deleted the move_mesh_type_to_grid branch August 22, 2025 12:17
@github-project-automation github-project-automation bot moved this from Backlog to Done in Parcels development Aug 22, 2025
erikvansebille added a commit that referenced this pull request Aug 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants