Skip to content

Conversation

@philipc2
Copy link
Member

Overview

  • We currently only support global HEALPix data, so it is necessary to add a safeguard to ensure that we don't round the zoom level.

@philipc2 philipc2 self-assigned this May 14, 2025
@philipc2 philipc2 added the bug Something isn't working label May 14, 2025
@philipc2 philipc2 requested a review from Copilot May 14, 2025 22:07
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 adds safeguards to ensure only global HEALPix data is processed by introducing a new helper function to calculate the zoom level from the number of cells. Key changes include:

  • Adding the get_zoom_from_cells function in uxarray/io/_healpix.py to validate global cell counts.
  • Replacing in-line logarithm calculations in uxarray/core/dataset.py and uxarray/core/dataarray.py with the new helper function.
  • Adding test coverage for invalid cell counts and updating API documentation.

Reviewed Changes

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

Show a summary per file
File Description
uxarray/io/_healpix.py Added new safeguard function for global HEALPix data validation.
uxarray/core/dataset.py Updated HEALPix grid creation to use the safeguard function.
uxarray/core/dataarray.py Updated HEALPix grid creation to use the safeguard function.
test/test_healpix.py Added tests to catch invalid cell count inputs.
docs/api.rst Updated API documentation to include the new from_healpix method.

# nearest integer
zoom = round(raw)
# verify exactness up to a tiny tolerance
if not raw == zoom:
Copy link

Copilot AI May 14, 2025

Choose a reason for hiding this comment

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

Floating point comparisons using '==' may fail due to precision issues. Consider using math.isclose(raw, zoom, rel_tol=1e-9) to allow for tolerance.

Suggested change
if not raw == zoom:
if not math.isclose(raw, zoom, rel_tol=1e-9):

Copilot uses AI. Check for mistakes.
Raises ValueError if log4(cells/12) is not an exact integer.
"""
# guard against non‐positive or non‐numeric input
ratio = float(cells) / 12.0
Copy link

Copilot AI May 14, 2025

Choose a reason for hiding this comment

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

Although the comment mentions guarding against non‐positive or non‐numeric input, no explicit check is performed. Consider adding input validation to prevent math domain errors when calculating the logarithm.

Copilot uses AI. Check for mistakes.
@philipc2 philipc2 merged commit e4ab53b into main May 15, 2025
20 checks passed
@erogluorhan erogluorhan deleted the healpix-warning branch September 26, 2025 17:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant