Skip to content

Add Sampled Cross-Sections #1321

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 36 commits into
base: main
Choose a base branch
from
Open

Add Sampled Cross-Sections #1321

wants to merge 36 commits into from

Conversation

philipc2
Copy link
Member

@philipc2 philipc2 commented Jul 20, 2025

Closes #1318 #1319 #1320 #1296

Overview

  • Adds a UxDataArray.cross_section() method, which can be used to sample along lines of constant lat/lon or arbitrary GCAs
  • Moves existing cross_section methods under the subset namespace and begins deprecation
  • Adds support for restricting the latitude or longitude range when doing a constant lat/lon subset
  • Safeguards cases where no faces are found when conducting a subset
  • Updates the Cross Section user guide notebook to use to showcase the new functionality, with vertical cross section examples.

@philipc2 philipc2 changed the title DRAFT: Support interpolation for cross-section outputs DRAFT: Support interpolating cross-section results Jul 20, 2025
@philipc2
Copy link
Member Author

@rajeeja

I'm opening up this PR to isolate our interpolation/sampling from the vertical cross sections examples in #1317

I've also put together a more general approach and cleaned up the implementation in #1317

@philipc2 philipc2 self-assigned this Jul 21, 2025
@philipc2
Copy link
Member Author

Sampling along a transect is very similar in concept to the work I put together in #1271, however instead of sampling onto pixels in an Image, we sample points along the transect.

@rajeeja
Copy link
Contributor

rajeeja commented Jul 21, 2025

Oh, if possible can you create a PR or changes directly to my PR. Are we planning to close it in favor of this?

@philipc2
Copy link
Member Author

Oh, if possible can you create a PR or changes directly to my PR. Are we planning to close it in favor of this?

Let's keep the API changes in this PR and use #1317 for applications to vertical cross sections (i.e. the updates to the notebooks, etc.)

Does that work? I'm happy to hop on sometime this week to pair program too.

Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@philipc2 philipc2 changed the title DRAFT: Support interpolating cross-section results DRAFT: Sampled cross-sections along GCAs, and lines of constant latitude/longitude Jul 23, 2025
@philipc2 philipc2 changed the title DRAFT: Sampled cross-sections along GCAs, and lines of constant latitude/longitude DRAFT: Add cross-sections Jul 23, 2025
@philipc2 philipc2 requested review from erogluorhan and rajeeja August 6, 2025 02:20
Copilot

This comment was marked as outdated.

@philipc2 philipc2 requested a review from Copilot August 6, 2025 02:24
Copilot

This comment was marked as outdated.

@philipc2 philipc2 requested a review from Copilot August 8, 2025 13:01
@philipc2 philipc2 changed the title Add cross-sections Add Sampled Cross-Sections Aug 8, 2025
Copilot

This comment was marked as outdated.

@philipc2 philipc2 requested a review from Copilot August 8, 2025 19:33
Copy link
Contributor

@Copilot 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 adds comprehensive cross-section functionality for sampling data along constant latitude/longitude lines and arbitrary great circle arcs. The changes consolidate cross-section methods under the subset namespace while deprecating existing methods and introduce a new sampled cross-section interface for data extraction.

  • Adds new UxDataArray.cross_section() method for sampling along arbitrary great circle arcs and constant lat/lon lines
  • Moves existing cross-section methods from cross_section to subset namespace with deprecation warnings
  • Implements constrained latitude/longitude range support for subset operations

Reviewed Changes

Copilot reviewed 8 out of 11 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
uxarray/subset/grid_accessor.py Added new constant_latitude/longitude methods and interval methods with enhanced error handling
uxarray/subset/dataarray_accessor.py Added new constant_latitude/longitude methods with lon/lat range constraints
uxarray/cross_sections/sample.py New file implementing sampling functions for geodesic, constant latitude, and constant longitude cross-sections
uxarray/cross_sections/grid_accessor.py Deprecated existing methods with warnings and redirects to subset accessor
uxarray/cross_sections/dataarray_accessor.py Replaced with new callable cross-section interface and deprecated old methods
test/test_cross_sections.py Updated tests to use subset namespace and added new cross-section functionality tests
docs/user-guide/cross-sections.ipynb Updated documentation to showcase new cross-section sampling functionality
docs/api.rst Updated API documentation to reflect new structure
Comments suppressed due to low confidence (1)


return self.uxda.isel(n_face=faces, inverse_indices=inverse_indices)

def constant_longitude_interval(
Copy link
Preview

Copilot AI Aug 8, 2025

Choose a reason for hiding this comment

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

The docstring contains a grammatical error: 'selecting all faces are within' should be 'selecting all faces that are within'

Copilot uses AI. Check for mistakes.

Comment on lines +145 to 146
__doc__ = __call__.__doc__

Copy link
Preview

Copilot AI Aug 8, 2025

Choose a reason for hiding this comment

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

Setting __doc__ to __call__.__doc__ creates a circular reference since __call__ doesn't have its own docstring. This should either be removed or set to the actual docstring content.

Suggested change
__doc__ = __call__.__doc__

Copilot uses AI. Check for mistakes.

Copy link
Contributor

@rajeeja rajeeja left a comment

Choose a reason for hiding this comment

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

looks good to go, minor fixes. in comments.

@philipc2 philipc2 requested a review from rajeeja August 18, 2025 17:23
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 great to me except a small comment I've put into the cross-sections notebook (can you see it?)

Copy link
Contributor

@rajeeja rajeeja left a comment

Choose a reason for hiding this comment

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

call still assumes 'n_face' exists, raise error here itself if not face-centered. Test for great clrcle case..

@njit(parallel=True)
def _fill_numba(flat_orig, face_idx, n_face, n_steps):
M = flat_orig.shape[0]
out = np.full((M, n_steps), np.nan, flat_orig.dtype)
Copy link
Contributor

Choose a reason for hiding this comment

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

np.nan cannot be represented in integer dtype; this might raise or produce invalid values.

@rajeeja rajeeja force-pushed the interpolate-cross-section branch from 95e4532 to 4d90463 Compare August 18, 2025 22:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants