Skip to content

Fix index parsing in mpas_to_xdmf tool#635

Merged
xylar merged 3 commits into
MPAS-Dev:masterfrom
xylar:fix-xdmf
May 29, 2025
Merged

Fix index parsing in mpas_to_xdmf tool#635
xylar merged 3 commits into
MPAS-Dev:masterfrom
xylar:fix-xdmf

Conversation

@xylar
Copy link
Copy Markdown
Collaborator

@xylar xylar commented May 29, 2025

This pull request focuses on improving the robustness of the _parse_indices function and significantly expanding the test coverage for the MPAS-to-XDMF conversion functionality. Key changes include handling edge cases in index parsing, adding validation, and introducing comprehensive unit tests for various scenarios.

Enhancements to _parse_indices function:

  • Improved handling of slice notation by validating the number of colons and padding missing parts to ensure consistent parsing. Added explicit error handling for invalid index strings.

Expanded test coverage:

  • Added unit tests for _parse_indices to validate both valid and invalid cases, including mixed slice/list formats, empty strings, and invalid strings.
  • Introduced tests for loading MPAS mesh datasets, handling time series, and processing extra dimensions, ensuring compatibility with edge cases like missing variables or invalid configurations.
  • Implemented tests for the CLI entry point (main) with simulated arguments and user input, verifying the creation of expected output files.

@xylar xylar requested a review from Copilot May 29, 2025 08:56
@xylar xylar self-assigned this May 29, 2025
Copy link
Copy Markdown
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 _parse_indices function to explicitly handle slice notation by splitting the index string into up to three parts, padding missing values, and applying defaults for start, stop, and step.

  • Replace one-liner list comprehension with explicit parsing for slice support
  • Preserve comma-separated index behavior unchanged
Comments suppressed due to low confidence (2)

conda_package/mpas_tools/viz/mpas_to_xdmf/io.py:300

  • Update the function docstring to describe the new slice notation support (start:stop:step semantics) and default behaviors when parts are omitted.
def _parse_indices(index_string, dim_size):

conda_package/mpas_tools/viz/mpas_to_xdmf/io.py:312

  • Add unit tests for _parse_indices covering edge cases like ':', '5:', ':10', '::2', and invalid formats to ensure the new parsing logic works as expected.
return list(range(start, stop, step))

Comment thread conda_package/mpas_tools/viz/mpas_to_xdmf/io.py
@xylar
Copy link
Copy Markdown
Collaborator Author

xylar commented May 29, 2025

Testing

I was able to extract kiteAreasOnVertex with vertexDegree=:, vertexDegree=0:3, and vertexDegree=0:3:1 with this fix, whereas it crashed without the fix.

xylar and others added 2 commits May 29, 2025 05:18
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@xylar xylar merged commit 1557e3f into MPAS-Dev:master May 29, 2025
6 checks passed
@xylar xylar deleted the fix-xdmf branch May 29, 2025 13:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants