Skip to content

Ensure freq_axis metadata propagation during NumPy ufunc evaluation#147

Open
vishwas-droid wants to merge 1 commit intosunpy:mainfrom
vishwas-droid:ufunc-freq-axis-clean
Open

Ensure freq_axis metadata propagation during NumPy ufunc evaluation#147
vishwas-droid wants to merge 1 commit intosunpy:mainfrom
vishwas-droid:ufunc-freq-axis-clean

Conversation

@vishwas-droid
Copy link

Spectrum objects were losing their freq_axis metadata when passed through NumPy ufuncs
(e.g., np.sqrt, np.log, arithmetic operations such as spec + 2).

Although the resulting object remained a Spectrum instance, the associated metadata
was silently dropped, which could lead to incorrect downstream scientific interpretation.

What This PR Does

This PR implements a custom __array_ufunc__ override to:

  • Execute NumPy ufuncs on plain ndarrays
  • Wrap the result back into a Spectrum
  • Safely propagate freq_axis metadata (deep copy)
  • Avoid recursion and ndarray subclass pitfalls

Tests

Regression tests have been added to ensure:

  • freq_axis is preserved after unary ufuncs (e.g., np.sqrt)
  • freq_axis is preserved after binary operations (e.g., spec + 5)

These tests prevent future silent metadata loss.

Rationale

Preserving metadata integrity is critical in scientific workflows, especially
for radio spectrogram analysis where frequency alignment is essential.

@samaloney
Copy link
Member

This is not specific to this PR. I'm posting this to all open PRs. Please be aware of the update to our AI usage policy, specifically on the disclosure of and acceptable uses of AI.

@samaloney
Copy link
Member

Is there an issue this PR is addressing? I'm not sure we want to support such operations of if this will be possible if/when we change the underling data model.

@vishwas-droid
Copy link
Author

Thanks for the question.

Yes — this PR addresses a concrete issue where freq_axis metadata was being silently dropped during NumPy ufunc evaluation (e.g., np.sqrt(spec) or spec + 2). Although the returned object remained a Spectrum, the associated metadata was lost, which could lead to subtle downstream inconsistencies.

The goal here is not to expand supported operations, but to preserve existing metadata for operations that already return valid Spectrum instances.

If the underlying data model changes in the future, this implementation can be adapted or removed accordingly — but currently it ensures consistency with user expectations and prevents silent metadata loss.

If this behavior is not something we want to guarantee long-term, I’m happy to adjust the scope or explore alternative approaches.

@samaloney
Copy link
Member

Sorry I meant an issue as in a GitHub issue not as in a code problem/bug. Generally requires PR to address open issue where the scope and direction of the solution can be discussed before any coding.

@vishwas-droid
Copy link
Author

Thanks for clarifying. I’ll open an issue to discuss the scope first and adjust the PR accordingly.

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.

2 participants

Comments