Skip to content

Conversation

@philipc2
Copy link
Member

@philipc2 philipc2 commented Jun 25, 2025

Overview

  • Adds support for reading node_edge_connectivity
  • Updates logic to improve readability

@philipc2 philipc2 self-assigned this Jun 25, 2025
@philipc2 philipc2 changed the title DRAFT: MPAS Reader Updates MPAS Reader Updates Jul 14, 2025
@philipc2 philipc2 marked this pull request as ready for review July 14, 2025 16:59

This comment was marked as outdated.

@philipc2 philipc2 requested a review from Copilot July 14, 2025 17:11
Copy link
Contributor

Copilot AI left a 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 enhances the MPAS reader by adding support for node-edge connectivity and refactoring mesh parsing functions for clarity.

  • Adds parsing and assignment of node_edge_connectivity and face_face_connectivity for the primal mesh.
  • Refactors several _parse_* functions to unify padding, zero-replacement, and zero-indexing logic.
  • Introduces a new test test_read_primal to verify connectivity variables.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
uxarray/io/_mpas.py Refactored connectivity parsers to generic routines; added node‐edge & face‐face parsing
test/test_mpas.py Added test_read_primal; test_read_dual currently lacks assertions
Comments suppressed due to low confidence (2)

test/test_mpas.py:34

  • The test_read_dual function has no assertions. Add checks for expected connectivity variables in the dual mesh (e.g., face_node_connectivity, node_edge_connectivity, etc.) to ensure the dual path is validated.
def test_read_dual():

uxarray/io/_mpas.py:366

  • [nitpick] The docstring specifies primal-only behavior but the signature implies general mesh support. Update the docstring to clearly state that this parser only handles the primal mesh or adjust behavior accordingly.
    """Parses face face connectivity for the primal mesh."""

).rename(dict(zip(edge_faces.dims, ugrid.EDGE_FACE_CONNECTIVITY_DIMS)))


def _parse_face_faces(in_ds, out_ds, mesh_type):
Copy link

Copilot AI Jul 14, 2025

Choose a reason for hiding this comment

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

The mesh_type parameter is not used inside this function. Consider removing it from the signature or extending the implementation to handle both primal and dual meshes.

Copilot uses AI. Check for mistakes.
Copy link
Member

@erogluorhan erogluorhan left a comment

Choose a reason for hiding this comment

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

This looks good to me except the single thing CoPilot has found.

@philipc2 philipc2 merged commit c35bd24 into main Aug 6, 2025
17 of 19 checks passed
@erogluorhan erogluorhan deleted the mpas-node-edge-conn branch September 26, 2025 17:47
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.

3 participants