Skip to content

Fix face_areas type hints and add compute_face_areas return type#1592

Merged
Sevans711 merged 1 commit into
UXARRAY:mainfrom
SAY-5:fix-face-areas-type-hints
Jul 20, 2026
Merged

Fix face_areas type hints and add compute_face_areas return type#1592
Sevans711 merged 1 commit into
UXARRAY:mainfrom
SAY-5:fix-face-areas-type-hints

Conversation

@SAY-5

@SAY-5 SAY-5 commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Closes #1590

Overview

  • calculate_total_face_area, compute_face_areas, and _compute_face_areas annotated quadrature_rule, order, and latitude_adjusted_area with | None, but None is never a valid value and the code does not handle it. Dropped the | None so the annotations match the actual accepted types (str, int, bool).
  • Added the missing return type annotation tuple[np.ndarray, np.ndarray] to compute_face_areas (and _compute_face_areas), matching what they return and the existing docstrings.

Kept narrowly scoped to the face_areas signatures per @Sevans711's note; the broader codebase-wide sweep is tracked separately.

Expected Usage

PR Checklist

General

  • An issue is linked created and linked
  • [N/A] Add appropriate labels
  • Filled out Overview and Expected Usage (if applicable) sections

Testing

  • [N/A] Adequate tests are created if there is new functionality
  • [N/A] Tests cover all possible logical paths in your function
  • [N/A] Tests are not too basic (such as simply calling a function and nothing else)

Documentation

  • [N/A] Docstrings have been added to all new functions
  • Docstrings have updated with any function changes
  • [N/A] Internal functions have a preceding underscore (_) and have been added to docs/internal_api/index.rst
  • [N/A] User functions have been added to docs/user_api/index.rst

Examples

  • [N/A] Any new notebook examples added to docs/examples/ folder
  • [N/A] Clear the output of all cells before committing
  • [N/A] New notebook files added to docs/examples.rst toctree
  • [N/A] New notebook files added to new entry in docs/gallery.yml with appropriate thumbnail photo in docs/_static/thumbnails/

Signed-off-by: Sai Asish Y <say.apm35@gmail.com>
@Sevans711

Copy link
Copy Markdown
Collaborator

Thank you @SAY-5 for taking the time to look into this and for making this clean pull request!

@rajeeja what would be your preferences on how to proceed? I think the fix looks good here when compared to main, but I know that the changes on #1577 would change the return type for compute_face_areas to something more complicated (something like: np.ndarray | UxDataArray | tuple[np.ndarray | UxDataArray, np.ndarray]). One option is to merge this fix to main as-is, and update the return type appropriately on #1577. Another option is to delay here and fix here after the other PR merges to main.

@rajeeja

rajeeja commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Thanks for the fix @SAY-5, and nice contribution for a first PR here. This resolves #1590 cleanly.

@Sevans711 — let's go ahead and merge this one now; I'll handle the small conflict with #1577 when I merge main in.

@SAY-5 out of curiosity, what got you into using uxarray? Always good to hear how people find the project.

@rajeeja rajeeja left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM — clean, correctly scoped fix. Approving.

@Sevans711
Sevans711 merged commit a5715ef into UXARRAY:main Jul 20, 2026
13 checks passed
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.

Fix type hints: compute_face_areas order param and return type

3 participants