Skip to content

Comments

swev-id: astropy__astropy-14995 Fix NDDataRef mask propagation when other operand has no mask#86

Open
casey-brooks wants to merge 3 commits intoastropy__astropy-14995from
fix/ndarithmetic-mask-bitwise_or-single-mask
Open

swev-id: astropy__astropy-14995 Fix NDDataRef mask propagation when other operand has no mask#86
casey-brooks wants to merge 3 commits intoastropy__astropy-14995from
fix/ndarithmetic-mask-bitwise_or-single-mask

Conversation

@casey-brooks
Copy link

@casey-brooks casey-brooks commented Dec 18, 2025

Issue

Reproduction

  1. In a clean checkout of astropy__astropy-14995, launch Python and run
    import numpy as np
    from astropy.nddata import NDDataRef
    
    mask = np.array([[True, False], [False, True]])
    data = np.array([[1, 2], [3, 4]])
    NDDataRef(data=data, mask=mask).multiply(1.0, handle_mask=np.bitwise_or)
  2. Swap operand order to exercise commutativity:
    import numpy as np
    from astropy.nddata import NDDataRef
    
    mask = np.array([[True, False], [False, True]])
    data = np.array([[1, 2], [3, 4]])
    masked = NDDataRef(data=data, mask=mask)
    unmasked = NDDataRef(data=data)
    masked.multiply(unmasked, handle_mask=np.bitwise_or)

Observed failure

Scalar operand with np.bitwise_or

/workspace/astropy/astropy/version.py:12: UserWarning: could not determine astropy package version; this indicates a broken installation
  warnings.warn(
Traceback (most recent call last):
  File "<stdin>", line 6, in <module>
  File "/workspace/astropy/astropy/nddata/mixins/ndarithmetic.py", line 618, in multiply
    return self._prepare_then_do_arithmetic(
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/workspace/astropy/astropy/nddata/mixins/ndarithmetic.py", line 731, in _prepare_then_do_arithmetic
    result, init_kwds = operand._arithmetic(operation, operand2, **kwargs)
                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/workspace/astropy/astropy/nddata/mixins/ndarithmetic.py", line 335, in _arithmetic
    kwargs["mask"] = self._arithmetic_mask(
                     ^^^^^^^^^^^^^^^^^^^^^^
  File "/workspace/astropy/astropy/nddata/mixins/ndarithmetic.py", line 527, in _arithmetic_mask
    return handle_mask(self.mask, operand.mask, **kwds)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
TypeError: unsupported operand type(s) for |: 'bool' and 'NoneType'

NDDataRef commutativity with np.bitwise_or

/workspace/astropy/astropy/version.py:12: UserWarning: could not determine astropy package version; this indicates a broken installation
  warnings.warn(
Traceback (most recent call last):
  File "<stdin>", line 7, in <module>
  File "/workspace/astropy/astropy/nddata/mixins/ndarithmetic.py", line 618, in multiply
    return self._prepare_then_do_arithmetic(
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/workspace/astropy/astropy/nddata/mixins/ndarithmetic.py", line 734, in _prepare_then_do_arithmetic
    result, init_kwds = cls._arithmetic(
                        ^^^^^^^^^^^^^^^^
  File "/workspace/astropy/astropy/nddata/mixins/ndarithmetic.py", line 335, in _arithmetic
    kwargs["mask"] = self._arithmetic_mask(
                     ^^^^^^^^^^^^^^^^^^^^^^
  File "/workspace/astropy/astropy/nddata/mixins/ndarithmetic.py", line 527, in _arithmetic_mask
    return handle_mask(self.mask, operand.mask, **kwds)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
TypeError: unsupported operand type(s) for |: 'bool' and 'NoneType'

Fix

  • Guard _arithmetic_mask against operand is None and capture the operand's mask before accessing it.
  • Return deep copies of whichever mask is present when the other operand lacks a mask so mask handlers never receive None inputs.
  • Preserve the existing handler path for the both-masked case.

Tests

  • Added regression tests covering scalar operands, NDDataRef commutativity, dual-mask combination, mask-less operands, and numpy array operands with both np.bitwise_or and np.logical_or handlers.
  • pytest astropy/nddata/mixins/tests/test_ndarithmetic.py -q
  • tox -e codestyle -- --files astropy/nddata/mixins/ndarithmetic.py astropy/nddata/mixins/tests/test_ndarithmetic.py

@casey-brooks casey-brooks requested a review from a team December 18, 2025 15:23
@casey-brooks casey-brooks changed the title temp swev-id: astropy__astropy-14995 Fix NDDataRef mask propagation when other operand has no mask Dec 18, 2025
@casey-brooks
Copy link
Author

Local Verification

  • pytest astropy/nddata/mixins/tests/test_ndarithmetic.py -q → 189 passed
  • tox -e codestyle -- --files astropy/nddata/mixins/ndarithmetic.py astropy/nddata/mixins/tests/test_ndarithmetic.py → passes

@rowan-stein rowan-stein added the skip-basebranch-check Skip base branch check in CI label Jan 6, 2026
Copy link

@noa-lucent noa-lucent left a comment

Choose a reason for hiding this comment

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

Thanks for the thorough regression coverage. The arithmetic mask logic now copies whichever mask is present so we no longer hand None to custom mask handlers, and the new tests exercise both scalar and NDDataRef operands. I left one minor cleanup request inline.

@casey-brooks
Copy link
Author

Test & Lint Summary

  • SETUPTOOLS_USE_DISTUTILS=stdlib PYTHONPATH=. LD_LIBRARY_PATH=/nix/store/wffgswxkp55xi14jy63rjsnfvl2qvmxy-gcc-14.3.0-lib/lib:/nix/store/qipd93x9gjyiygqk673rd2ssnf8y7jj0-gcc-14.3.0-lib/lib:/nix/store/zmjqwzhgl9hwr8xnk8raj8d50lzkql66-gcc-14.3.0-lib/lib:/nix/store/n5lymg0y5x6i9wipkjrsi8aczv1nr4qc-zlib-1.3.1/lib${LD_LIBRARY_PATH:+:$LD_LIBRARY_PATH} .venv/bin/python -m pytest astropy/nddata/mixins/tests/test_ndarithmetic.py -q
    • result: 189 passed (UserWarning: could not determine astropy package version)
  • SETUPTOOLS_USE_DISTUTILS=stdlib LD_LIBRARY_PATH=/nix/store/wffgswxkp55xi14jy63rjsnfvl2qvmxy-gcc-14.3.0-lib/lib:/nix/store/qipd93x9gjyiygqk673rd2ssnf8y7jj0-gcc-14.3.0-lib/lib:/nix/store/zmjqwzhgl9hwr8xnk8raj8d50lzkql66-gcc-14.3.0-lib/lib:/nix/store/n5lymg0y5x6i9wipkjrsi8aczv1nr4qc-zlib-1.3.1/lib${LD_LIBRARY_PATH:+:$LD_LIBRARY_PATH} .venv/bin/tox -e codestyle -- --files astropy/nddata/mixins/ndarithmetic.py astropy/nddata/mixins/tests/test_ndarithmetic.py
    • result: passed

Copy link

@noa-lucent noa-lucent left a comment

Choose a reason for hiding this comment

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

Thanks for the quick cleanup. The stray comment is gone and everything else still looks good. Approved.

Copy link
Collaborator

@rowan-stein rowan-stein left a comment

Choose a reason for hiding this comment

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

Approve: mask propagation fixes validated; tests and codestyle passed.

@rowan-stein rowan-stein changed the base branch from astropy__astropy-14995 to main January 11, 2026 18:37
@rowan-stein rowan-stein changed the base branch from main to astropy__astropy-14995 January 12, 2026 20:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

skip-basebranch-check Skip base branch check in CI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

NDDataRef mask propagation fails in v5.3 when operand has no mask (bitwise_or)

3 participants