Skip to content

Preserve floating-point dtype in wrap_lons (#4119)#7119

Open
gaoflow wants to merge 1 commit into
SciTools:mainfrom
gaoflow:fix/wrap-lons-preserve-dtype
Open

Preserve floating-point dtype in wrap_lons (#4119)#7119
gaoflow wants to merge 1 commit into
SciTools:mainfrom
gaoflow:fix/wrap-lons-preserve-dtype

Conversation

@gaoflow

@gaoflow gaoflow commented Jun 1, 2026

Copy link
Copy Markdown

🚀 Pull Request

Description

Closes #4119.

iris.analysis.cartography.wrap_lons unconditionally cast its result to float64:

lons = lons.astype(np.float64)
return ((lons - base) % period) + base

so float32 (and float16) longitudes were silently promoted to float64. This is wasteful for memory/IO and surprising when a coordinate's dtype changes purely as a side effect of wrapping. As noted in #4119, the 64-bit cast is only needed internally for precision during the range shift — the final result can be returned in the input's original floating dtype.

Reproduction (before the fix)

>>> import numpy as np
>>> from iris.analysis.cartography import wrap_lons
>>> wrap_lons(np.array([185, 30, -200, 75], dtype=np.float32), -180, 360).dtype
dtype('float64')   # promoted!

After the fix

>>> wrap_lons(np.array([185, 30, -200, 75], dtype=np.float32), -180, 360).dtype
dtype('float32')   # preserved

Approach

The 64-bit computation is kept (precision during the modulo), but the result is cast back to the input dtype only for floating-point inputs (dtype.kind == "f"). Integer (and other non-floating) inputs still return float64, which preserves the documented docstring example wrap_lons(np.array([185, 30, -200, 75]), -180, 360)[-175. 30. 160. 75.] (int in, float64 out). Laziness (dask) and masking are preserved unchanged.

Verification

  • New unit test tests/unit/analysis/cartography/test_wrap_lons.py covering float16/float32/float64 (preserved), int32/int64 (→ float64), masked arrays, and lazy dask input (stays lazy + dtype). The dtype-preservation cases fail on main and pass with this change.
  • Existing cube intersection unit tests (142), test_intersect integration tests, and the cartography unit suite all pass.
  • ruff check / ruff format clean.

Note

This revisits the change abandoned in #4138 (closed for non-technical reasons). Per @rcomer's suggestion there, this is a fresh branch/PR. Unlike the earlier sketch, this version preserves only the floating dtype, so the integer docstring example is unaffected.

iris.analysis.cartography.wrap_lons always cast its result to float64,
so float32 (or float16) longitudes were silently promoted to float64.
Preserve the original dtype for floating-point inputs while still
returning float64 for integer inputs (matching the documented example),
and keep laziness and masking intact.

Fixes SciTools#4119.
@CLAassistant

CLAassistant commented Jun 1, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@trexfeathers

Copy link
Copy Markdown
Contributor

Hi @gaoflow, thanks for your avalanche of contributions today 🙏

These contributions bear some markers of being written by LLM agents. Can you confirm? How was your experience? How much code did you need to edit yourself? Did you deliberately target the Good First Issue A good issue to take on if you're just getting started with Iris development label?

Our core development team are all assigned to other work right now, but in two weeks' time we are hoping to spend several days training on agentic programming. These pull requests will make a great example! So I'd be keen to keep them open until then; can you wait two weeks for a review?

@gaoflow

gaoflow commented Jun 1, 2026

Copy link
Copy Markdown
Author

Hi @trexfeathers, thanks for the warm welcome and for taking the time to ask directly.

Yes — I can confirm these were written with the help of an AI coding agent (Claude Code, Opus 4.8). My workflow for each one was to have it locate a self-contained, reproducible bug, reproduce it locally, write the fix plus a regression test (verified to fail on main and pass with the change), and run the relevant existing test suites to check for regressions. I reviewed and verified each diff and the reproduction myself before opening the PR rather than editing the code heavily by hand.

On the Good First Issue label: I wasn't deliberately filtering by it — I was selecting issues that had a clear reproduction and a contained fix, and a few of those happen to carry that label.

Two weeks is completely fine — please take whatever time you need, and I'm glad if they're useful for your agentic-programming training. Happy to answer any questions or adjust any of the PRs in the meantime.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

Preserve dtype for wrap_lons

3 participants