fix: preserve masked array in view

[flying-sheep]

• Closes #
• Tests added

• Release note not necessary because:

---

_{Stack created with GitHub Stacks CLI • Give Feedback 💬}

[codecov]

Codecov Report

❌ Patch coverage is 92.59259% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 85.72%. Comparing base (52557f3) to head (9bacef5).
✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
src/anndata/_core/views.py	91.78%	6 Missing ⚠️

Additional details and impacted files

@@                Coverage Diff                 @@
##           fix-array-view    #2582      +/-   ##
==================================================
+ Coverage           85.66%   85.72%   +0.05%     
==================================================
  Files                  49       49              
  Lines                8072     8146      +74     
==================================================
+ Hits                 6915     6983      +68     
- Misses               1157     1163       +6     

Files with missing lines	Coverage Δ
src/anndata/_core/merge.py	85.77% <100.00%> (+0.11%)	⬆️
src/anndata/_io/specs/methods.py	91.80% <100.00%> (+0.02%)	⬆️
src/anndata/_core/views.py	87.20% <91.78%> (+0.87%)	⬆️

[selmanozleyen]

Maybe we can give a warning when we modify a view of a view e.g. sub = adata[:5, :].obsm["masked"][1:3][0,0] = 1? Because this didn't use to modify the source no? So it's a behavioural change.

I checked and it can be done with

def __getitem__(self, index):
    result = super().__getitem__(index)
    if isinstance(result, MaskedArrayView):
        result._view_args = self._view_args
    return result

This way we also stay consistent with ArrayView

[flying-sheep]

OK, this should do the trick

[selmanozleyen]

Besides my note,

But I found a prexisting old bug lol. But I don't think it belongs here because it also exists for ArrayView.

import warnings

import numpy as np
import pytest

import anndata as ad
from anndata import ImplicitModificationWarning


def test_reslice_write_hits_correct_cell():
    # obsm element is a 5x4 arange, so element[r, c] == r*4 + c
    a = ad.AnnData(
        np.zeros((10, 10)),
        obsm={"o": np.arange(40.0).reshape(10, 4)},
    )

    view = a[:5, :]
    element = view.obsm["o"]          # ArrayView, rows 0..4
    sub = element[1:3]                # rows 1..2  ->  sub[0] IS element row 1

    # sub[0, 0] currently equals element[1, 0] == 4.0
    assert sub[0, 0] == 4.0

    # Write 777 to sub[0, 0]. It should update the view's row 1, col 0.
    with pytest.warns(ImplicitModificationWarning):
        sub[0, 0] = 777.0

    actualized = np.asarray(view.obsm["o"])

    # Copy-on-write DID protect the top-level original — this part is fine:
    assert a.obsm["o"][1, 0] == 4.0, "parent must not be mutated"

    # BUG: the write should land at row 1, col 0 (the cell sub[0,0] refers to),
    # but it actually lands at row 0, col 0.
    assert actualized[1, 0] == 777.0, (
        f"expected 777 at row 1 (the sub-slice's row 0), "
        f"but row1={actualized[1, 0]}, row0={actualized[0, 0]}"
    )
    assert actualized[0, 0] == 0.0, "row 0 should have been left untouched"

[flying-sheep]

I addressed your note, just not by overriding that function! Instead I overrode it farther up.

I think we should probably fix that old bug independently first then!

[selmanozleyen]

I agree the fix should be an independent PR. I didn't understand why .pre-commit-config.yaml but the rest looks good to me!

[flying-sheep]

I didn't understand why .pre-commit-config.yaml but the rest looks good to me!

just some obsolete redundant stuff that I forgot to get rid of elsewhere.

[review-notebook-app]

Check out this pull request on 

See visual diffs & provide feedback on Jupyter Notebooks.

---

Powered by ReviewNB

[selmanozleyen]

So honestly my review was just focused on automized comparison vs main. It seems like the new pr on top fixes some of the things I mentioned but I made another claude run to identify what's regressing and what isn't. I also attached it's suggestion but it isn't mine.

So here is the AI report as is

Regressions vs main

#	Behaviour	main	this stack
1	write_h5ad/write_zarr of a view with masked X/layers/obsm	OK (6/6)	IORegistryError (6/6)
2	pickle(view.obsm["masked"])	OK → ArrayView	TypeError
3	ad.concat([a[:5], a[5:]])	ndarray, setitem OK, write OK	MaskedArrayView, setitem AttributeError, write IORegistryError
4	AnnData(obsm={"m": view_el + 1})	ndarray, writable	MaskedArrayView, IORegistryError
5	write via .T/.reshape/.ravel/.swapaxes/.squeeze/.transpose/.real	warned + actualized, parent safe	no warning, not actualized, parent silently mutated

1. The IO registry dispatches on exact type(elem) (registry.py:242, :355),
and MaskedArrayView is unregistered — so every write of a view holding a masked
array fails, both formats, all three slots.

5 — the one I'd worry about most. numpy.ma's _arraymethod wrappers build
the result from self._data and restore state via _update_from, which knows
nothing about _view_args, so it comes back None. Copy-on-write is then skipped
entirely and the write goes straight into the parent's buffer. main is also
wrong here (it writes to the wrong cell) but it warns and actualizes, so the
source object is never touched. This breaks the copy-on-write contract itself.

3 has a second root cause: np.ma.concatenate preserves the subclass and
constructs via ndarray.__new__, so __array_finalize__ receives obj=None, and
the if obj is not None guard leaves _view_args never assigned — hence an
AttributeError on a plain, non-view AnnData.

New defect (not strictly a regression)

view.obsm["m"].mask = np.ones(...) → no warning, view not actualized, parent's
mask silently overwritten. On main .mask didn't exist on an ArrayView
(AttributeError), so no data could be lost. The setter at views.py:269 calls
__setmask__ unconditionally, while __setitem__ directly above it routes
through view_update.

Pre-existing, unchanged — but now inconsistent

Derived-index writes land on the wrong cells identically on main and here:
el[1:3][0,0] = x hits row 0, and el[:, 0][3] = x overwrites all five cells of
row 3. #2596 fixes exactly this, but MaskedArrayView never inherits the
_is_element guard, so within this branch:

obsm['plain'][1:3]   -> ValueError: Cannot modify `.obsm` of a view through a derived array
obsm['masked'][1:3]  -> silently written to the wrong cell

Two other pre-existing issues this PR now runs into:

• zarr v3 cannot encode np.ma.MaskedArray. write_zarr of a non-view
  AnnData with a masked obsm already fails on main:
  ValueError: cannot reshape array of size 50 into shape (400,), from zarr's
  BytesCodec._encode_sync doing nd_array.ravel().view(dtype="B"), which trips
  np.ma's dtype setter reshaping _mask. zarr v2 and h5ad are fine. Dropping
  the mask in views was accidentally shielding views from this, so fixing
  regression 1 alone just turns the IORegistryError into this ValueError on v3.
• The mask is dropped on write anyway — an h5ad round-trip returns a plain
  ndarray with the masked cell's raw value. There's no masked-array IOSpec, so
  "preserve MaskedArray in views" holds in memory only. Probably the same open
  question as the # TODO: Is this the right behavior for MaskedArrays? at
  methods.py:474 — worth settling explicitly, since it decides whether any of
  this should round-trip at all.

Why CI is green

tests/test_views.py is the only file in the repo that mentions np.ma at all, so
nothing exercises masked arrays through IO, concat, or pickle. The 5 new tests all
use obsm (never X or layers, both equally affected), and none covers derived
arrays — despite this being stacked directly on the PR that added that guard.

Fix direction (from the AI run — not my proposal)

Making MaskedArrayView inherit ArrayView
(class MaskedArrayView(ArrayView, np.ma.MaskedArray); the MRO is valid) resolves
regressions 2–5 plus the consistency gap, given four supporting changes:

• make ArrayView.__array_finalize__ cooperative
  (super().__array_finalize__(obj)) — without this MaskedArray.__array_finalize__
  never runs, _mask is never initialised, and the mask is silently lost again
• drop the if obj is not None guard so _view_args is always assigned
• override _update_from to carry _view_args and reset _is_element
• add __reduce__ returning a plain masked array

plus two registry entries. The zarr v3 bug is independent.

Verification

	this head	with the patch below
tests/test_views_masked.py	37 failed, 24 passed, 7 xfailed	5 failed, 56 passed, 7 xfailed

The 5 survivors are real residual gaps the patch doesn't attempt: the type leak
through np.ma.concatenate, .real, np.ma arithmetic not routing through
__array_ufunc__, and nomask. Existing suites stay green with it —
test_views.py, test_io_elementwise.py, test_concatenate.py → 4373 passed.

tests/test_views_masked.py — 61 tests reproducing everything above

"""Tests for `numpy.ma.MaskedArray` elements in views.

Every test here asserts the *desired* behaviour, so the ones that currently fail
mark real defects in `MaskedArrayView` (:pr:`2582`). Each is annotated with how
it behaves on `main` so regressions can be told apart from pre-existing bugs.
"""

from __future__ import annotations

import contextlib
import pickle
import warnings
from typing import TYPE_CHECKING

import numpy as np
import pytest

import anndata as ad
from anndata import ImplicitModificationWarning
from anndata._core.views import ArrayView, MaskedArrayView

if TYPE_CHECKING:
    from collections.abc import Callable
    from pathlib import Path
    from typing import Literal

N_OBS, N_VARS, N_DIM = 10, 10, 5
"""Shapes: `X` is ``(N_OBS, N_VARS)``, the masked payload ``(N_OBS, N_DIM)``."""

ZARR_V3_MASKED = (
    "pre-existing, unrelated to views: zarr v3 cannot encode np.ma.MaskedArray, "
    "because BytesCodec._encode_sync does .ravel().view(dtype='B') and np.ma's "
    "dtype setter then fails to reshape _mask. zarr v2 and h5ad are fine."
)


def masked_payload() -> np.ma.MaskedArray:
    """``arange`` data with a single masked cell at ``[0, 0]``."""
    mask = np.zeros((N_OBS, N_DIM), dtype=bool)
    mask[0, 0] = True
    data = np.arange(float(N_OBS * N_DIM)).reshape(N_OBS, N_DIM)
    return np.ma.MaskedArray(data, mask=mask)


def mk(where: Literal["obsm", "layers", "X"] = "obsm") -> ad.AnnData:
    """An AnnData holding `masked_payload` in `where`."""
    payload = masked_payload()
    match where:
        case "obsm":
            return ad.AnnData(np.zeros((N_OBS, N_VARS)), obsm={"masked": payload})
        case "layers":
            return ad.AnnData(np.zeros((N_OBS, N_DIM)), layers={"masked": payload})
        case "X":
            return ad.AnnData(payload)
        case _:  # pragma: no cover
            pytest.fail(f"unknown slot {where!r}")


def get(adata: ad.AnnData, where: str) -> np.ndarray:
    return adata.X if where == "X" else getattr(adata, where)["masked"]


@pytest.fixture(params=["obsm", "layers", "X"])
def where(request: pytest.FixtureRequest) -> str:
    """Which slot holds the masked array. All three take the same code path."""
    return request.param


# --------------------------------------------------------------------------- #
# what the PR sets out to do
# --------------------------------------------------------------------------- #


def test_view_preserves_mask(where: str) -> None:
    """The point of :pr:`2582`. Fails on `main`, where the mask is dropped."""
    el = get(mk(where)[:5, :], where)

    assert isinstance(el, MaskedArrayView)
    assert np.ma.is_masked(el)
    assert np.ma.getmaskarray(el)[0, 0]
    assert not np.ma.getmaskarray(el)[1, 0]


# --------------------------------------------------------------------------- #
# writes through the element itself must land on the addressed cells
# --------------------------------------------------------------------------- #


def test_element_write_placement(where: str) -> None:
    adata = mk(where)
    view = adata[:5, :]

    with pytest.warns(ImplicitModificationWarning, match=where):
        get(view, where)[1, 0] = 1234.0

    assert get(view, where)[1, 0] == 1234.0
    # copy-on-write: the source is untouched
    assert get(adata, where)[1, 0] == N_DIM


def test_mask_write_placement(where: str) -> None:
    """`.mask[…] = …` must actualize and hit the addressed cell."""
    adata = mk(where)
    view = adata[:5, :]

    with pytest.warns(ImplicitModificationWarning, match=where):
        get(view, where).mask[1, 0] = True

    assert np.ma.getmaskarray(get(view, where))[1, 0]
    assert not np.ma.getmaskarray(get(adata, where))[1, 0]


def test_data_write_placement(where: str) -> None:
    """`.data[…] = …` must actualize and hit the addressed cell."""
    adata = mk(where)
    view = adata[:5, :]
    mask_before = np.ma.getmaskarray(get(adata, where)).copy()

    with pytest.warns(ImplicitModificationWarning, match=where):
        get(view, where).data[2, 0] = 999.0

    assert np.ma.getdata(get(view, where))[2, 0] == 999.0
    assert np.ma.getdata(get(adata, where))[2, 0] == 2 * N_DIM
    np.testing.assert_array_equal(np.ma.getmaskarray(get(adata, where)), mask_before)


def test_element_write_placement_view_of_view() -> None:
    adata = mk()
    view = adata[2:8][1:4]  # original rows 3, 4, 5

    np.testing.assert_array_equal(
        np.ma.getdata(view.obsm["masked"])[:, 0], [15.0, 20.0, 25.0]
    )
    with pytest.warns(ImplicitModificationWarning, match="obsm"):
        view.obsm["masked"][0, 0] = -1.0

    np.testing.assert_array_equal(
        np.ma.getdata(view.obsm["masked"])[:, 0], [-1.0, 20.0, 25.0]
    )
    assert np.ma.getdata(adata.obsm["masked"])[3, 0] == 15.0


def test_mask_write_placement_fancy_index() -> None:
    adata = mk()
    view = adata[[9, 0, 5], :]  # original rows 9, 0, 5; row 0 is the masked one

    with pytest.warns(ImplicitModificationWarning, match="obsm"):
        view.obsm["masked"].mask[0, 0] = True

    # newly masked row 0, plus the original mask carried on row 1 (= original 0)
    np.testing.assert_array_equal(
        np.ma.getmaskarray(view.obsm["masked"])[:, 0], [True, True, False]
    )
    np.testing.assert_array_equal(
        np.ma.getmaskarray(adata.obsm["masked"])[:, 0], [True] + [False] * 9
    )


# --------------------------------------------------------------------------- #
# derived arrays: `idx` addresses the derivative, not the element, so the
# copy-on-modify in `_SetItemMixin` cannot place the write. `ArrayView` raises
# (:pr:`2596`); `MaskedArrayView` must do the same.
# --------------------------------------------------------------------------- #

type _Idx = int | tuple[int, int]

DERIVED: list[tuple[str, Callable[[np.ndarray], np.ndarray], _Idx]] = [
    ("getitem_slice", lambda e: e[1:3], (0, 0)),
    ("getitem_column", lambda e: e[:, 0], 3),
    ("T", lambda e: e.T, (1, 0)),
    ("transpose", lambda e: e.transpose(), (1, 0)),
    ("reshape", lambda e: e.reshape(N_DIM * N_DIM), 6),
    ("ravel", lambda e: e.ravel(), 6),
    ("swapaxes", lambda e: e.swapaxes(0, 1), (1, 0)),
    ("squeeze", lambda e: e.squeeze(), (1, 1)),
    ("view", lambda e: e.view(), (1, 1)),
    ("real", lambda e: e.real, (1, 1)),
]


@pytest.mark.parametrize(
    ("derive", "idx"), [p[1:] for p in DERIVED], ids=[p[0] for p in DERIVED]
)
def test_derived_write_raises(
    derive: Callable[[np.ndarray], np.ndarray], idx: _Idx
) -> None:
    """Same contract `ArrayView` got in :pr:`2596`."""
    view = mk()[:5, :]
    derived = derive(view.obsm["masked"])

    with pytest.raises(ValueError, match=r"through a derived array"):
        derived[idx] = -1.0


@pytest.mark.parametrize(
    ("derive", "idx"), [p[1:] for p in DERIVED], ids=[p[0] for p in DERIVED]
)
def test_derived_write_never_mutates_parent(
    derive: Callable[[np.ndarray], np.ndarray], idx: _Idx
) -> None:
    """The copy-on-write invariant: a write through a view never reaches its source.

    Weaker than `test_derived_write_raises` on purpose — it still holds if the
    write is allowed, as long as the view is actualized first. On `main` every
    case passes this (wrong cell, but the source is safe); on :pr:`2582` the
    `numpy.ma` `_arraymethod` derivatives (`.T`, `.reshape`, …) lose
    `_view_args` and write straight into the source buffer.
    """
    adata = mk()
    view = adata[:5, :]
    data_before = np.ma.getdata(adata.obsm["masked"]).copy()
    mask_before = np.ma.getmaskarray(adata.obsm["masked"]).copy()

    derived = derive(view.obsm["masked"])
    with (
        warnings.catch_warnings(),
        # refusing the write outright is the preferred outcome, see above
        contextlib.suppress(ValueError),
    ):
        warnings.simplefilter("ignore", ImplicitModificationWarning)
        derived[idx] = -1.0

    np.testing.assert_array_equal(
        np.ma.getdata(adata.obsm["masked"]),
        data_before,
        err_msg="source AnnData data was mutated through a view",
    )
    np.testing.assert_array_equal(
        np.ma.getmaskarray(adata.obsm["masked"]),
        mask_before,
        err_msg="source AnnData mask was mutated through a view",
    )


@pytest.mark.parametrize("attr", ["mask", "data"])
def test_derived_subarray_write_raises(attr: str) -> None:
    """`.mask[1:3]` / `.data[1:3]` are derivatives too."""
    view = mk()[:5, :]
    derived = getattr(view.obsm["masked"], attr)[1:3]

    with pytest.raises(ValueError, match=r"through a derived array"):
        derived[0, 0] = True if attr == "mask" else -1.0


def test_derived_write_consistent_with_plain_array() -> None:
    """A masked element must not be more permissive than a plain one."""
    plain = np.arange(float(N_OBS * N_DIM)).reshape(N_OBS, N_DIM)
    adata = ad.AnnData(
        np.zeros((N_OBS, N_VARS)),
        obsm={"plain": plain, "masked": masked_payload()},
    )
    view = adata[:5, :]

    assert isinstance(view.obsm["plain"], ArrayView)
    with pytest.raises(ValueError, match=r"through a derived array"):
        view.obsm["plain"][1:3][0, 0] = -1.0
    with pytest.raises(ValueError, match=r"through a derived array"):
        view.obsm["masked"][1:3][0, 0] = -1.0


# --------------------------------------------------------------------------- #
# assigning a whole new mask
# --------------------------------------------------------------------------- #


def test_mask_setter_is_copy_on_write(where: str) -> None:
    """`el.mask = …` must go through `view_update` like `el[…] = …` does."""
    adata = mk(where)
    view = adata[:5, :]
    before = np.ma.getmaskarray(get(adata, where)).copy()

    with pytest.warns(ImplicitModificationWarning, match=where):
        get(view, where).mask = np.ones((5, N_DIM), dtype=bool)

    assert np.ma.getmaskarray(get(view, where)).all()
    np.testing.assert_array_equal(
        np.ma.getmaskarray(get(adata, where)),
        before,
        err_msg="source AnnData mask was overwritten through a view",
    )


def test_mask_setter_on_non_view() -> None:
    """Control: outside a view the setter is a plain `__setmask__`."""
    adata = mk()
    adata.obsm["masked"].mask = np.ones((N_OBS, N_DIM), dtype=bool)

    assert np.ma.getmaskarray(adata.obsm["masked"]).all()


# --------------------------------------------------------------------------- #
# the view type must not escape into detached arrays
# --------------------------------------------------------------------------- #


def test_ufunc_result_is_not_a_view() -> None:
    """`ArrayView.__array_ufunc__` returns plain arrays; masked must match."""
    el = mk()[:5, :].obsm["masked"]

    for result in (el + 1, el * 2, np.sqrt(el), el.sum(axis=0)):
        assert not isinstance(result, MaskedArrayView), type(result)


def test_detached_result_is_storable(tmp_path: Path) -> None:
    """A computed array must be usable as a normal AnnData element."""
    adata = ad.AnnData(np.zeros((5, 3)), obsm={"m": mk()[:5, :].obsm["masked"] + 1})

    adata.write_h5ad(tmp_path / "detached.h5ad")


def test_concat_of_views_does_not_leak_view_type() -> None:
    """`np.ma.concatenate` keeps the subclass, so `concat` must not hand it one."""
    adata = mk()
    with warnings.catch_warnings():
        warnings.simplefilter("ignore", UserWarning)  # non-unique obs_names
        result = ad.concat([adata[:5], adata[5:]])

    assert not result.is_view
    assert not isinstance(result.obsm["masked"], MaskedArrayView)


def test_concat_of_views_result_is_writable() -> None:
    """A `MaskedArrayView` built by `ndarray.__new__` never gets `_view_args`."""
    adata = mk()
    with warnings.catch_warnings():
        warnings.simplefilter("ignore", UserWarning)
        result = ad.concat([adata[:5], adata[5:]])

    result.obsm["masked"][0, 0] = 1.0  # must not raise AttributeError
    assert np.ma.getdata(result.obsm["masked"])[0, 0] == 1.0


# --------------------------------------------------------------------------- #
# copy / pickle
# --------------------------------------------------------------------------- #


def test_copy_is_detached_masked_array() -> None:
    el = mk()[:5, :].obsm["masked"]

    copied = el.copy()
    assert type(copied) is np.ma.MaskedArray
    assert not np.shares_memory(copied, el)
    np.testing.assert_array_equal(np.ma.getmaskarray(copied), np.ma.getmaskarray(el))


def test_pickle_roundtrip() -> None:
    """`np.ma`'s reconstructor calls `cls.__new__(cls, data, mask=…, dtype=…)`."""
    el = mk()[:5, :].obsm["masked"]

    restored = pickle.loads(pickle.dumps(el))

    assert np.ma.is_masked(restored)
    np.testing.assert_array_equal(np.ma.getmaskarray(restored), np.ma.getmaskarray(el))
    np.testing.assert_array_equal(np.ma.getdata(restored), np.ma.getdata(el))


def test_adata_view_copy_keeps_mask(where: str) -> None:
    copied = mk(where)[:5, :].copy()

    assert np.ma.is_masked(get(copied, where))
    assert np.ma.getmaskarray(get(copied, where))[0, 0]


# --------------------------------------------------------------------------- #
# IO
# --------------------------------------------------------------------------- #


def test_write_view_with_masked(
    tmp_path: Path,
    diskfmt: Literal["h5ad", "zarr"],
    where: str,
    request: pytest.FixtureRequest,
) -> None:
    """The IO registry dispatches on exact `type(elem)`, so views need registering."""
    if diskfmt == "zarr" and ad.settings.zarr_write_format == 3:
        request.applymarker(pytest.mark.xfail(reason=ZARR_V3_MASKED, strict=False))
    view = mk(where)[:5, :]

    getattr(view, f"write_{diskfmt}")(tmp_path / f"view.{diskfmt}")


def test_write_non_view_masked(
    tmp_path: Path, diskfmt: Literal["h5ad", "zarr"], request: pytest.FixtureRequest
) -> None:
    """Baseline for the test above: does the *non*-view case work at all?

    It does, except on zarr v3 — which is why the view case failing is on
    :pr:`2582` and not on the format.
    """
    if diskfmt == "zarr" and ad.settings.zarr_write_format == 3:
        request.applymarker(pytest.mark.xfail(reason=ZARR_V3_MASKED, strict=False))

    getattr(mk(), f"write_{diskfmt}")(tmp_path / f"plain.{diskfmt}")


def test_write_elem_view_element(tmp_path: Path) -> None:
    """`write_elem` on the element directly, without an enclosing AnnData."""
    import h5py

    el = mk()[:5, :].obsm["masked"]
    with h5py.File(tmp_path / "el.h5", "w") as f:
        ad.io.write_elem(f, "el", el)


def test_write_concat_result(tmp_path: Path) -> None:
    adata = mk()
    with warnings.catch_warnings():
        warnings.simplefilter("ignore", UserWarning)
        result = ad.concat([adata[:5], adata[5:]])

    result.write_h5ad(tmp_path / "concat.h5ad")


@pytest.mark.xfail(
    reason="pre-existing: no masked-array IOSpec, the mask is dropped on write",
    strict=False,
)
def test_mask_roundtrips(tmp_path: Path) -> None:
    """Nothing persists the mask, so `MaskedArray` is in-memory-only for now."""
    mk().write_h5ad(tmp_path / "rt.h5ad")

    restored = ad.read_h5ad(tmp_path / "rt.h5ad")
    assert np.ma.is_masked(restored.obsm["masked"])


# --------------------------------------------------------------------------- #
# smaller gaps
# --------------------------------------------------------------------------- #


@pytest.mark.xfail(
    reason="pre-existing: `ArrayView.copy` drops `order` too (it returns "
    "`np.array(self)`), so this is a shared gap rather than a regression",
    strict=False,
)
def test_copy_honours_order() -> None:
    el = mk()[:5, :].obsm["masked"]

    assert el.copy(order="F").flags["F_CONTIGUOUS"]


def test_nomask_mask_write() -> None:
    """With `nomask`, `.mask` is `np.False_`, so item assignment has nothing to hit."""
    adata = ad.AnnData(
        np.zeros((N_OBS, N_VARS)),
        obsm={"m": np.ma.MaskedArray(np.zeros((N_OBS, N_DIM)))},
    )
    view = adata[:5, :]

    with pytest.warns(ImplicitModificationWarning, match="obsm"):
        view.obsm["m"].mask[0, 0] = True

    assert np.ma.getmaskarray(view.obsm["m"])[0, 0]
    assert not np.ma.getmaskarray(adata.obsm["m"])[0, 0]


@pytest.mark.xfail(
    reason="pre-existing: `main` downgrades the subclass to `ArrayView`, so this "
    "is a long-standing gap; `np.ma.asarray` in `__new__` is `subok=False`",
    strict=False,
)
def test_view_retains_masked_subclass() -> None:
    """`np.ma.asarray` is `subok=False`; cf. `test_view_retains_ndarray_subclass`."""

    class MaskedSubclass(np.ma.MaskedArray):
        pass

    payload = MaskedSubclass(masked_payload())
    adata = ad.AnnData(np.zeros((N_OBS, N_VARS)), obsm={"masked": payload})

    assert isinstance(adata[:5, :].obsm["masked"], MaskedSubclass)

Suggested patch (illustrative sketch, not a proposal)

diff --git a/src/anndata/_core/views.py b/src/anndata/_core/views.py
index 99e390af..cfd13725 100644
--- a/src/anndata/_core/views.py
+++ b/src/anndata/_core/views.py
@@ -143,11 +143,11 @@ class ArrayView(_SetItemMixin, np.ndarray):
         return arr
 
     def __array_finalize__(self, obj: np.ndarray | None) -> None:
+        super().__array_finalize__(obj)
         # Derivatives (e.g. `el.T`, `el[1:3]`, …) inherit `_view_args`,
         # but aren’t what it refers to
         self._is_element = False
-        if obj is not None:
-            self._view_args = getattr(obj, "_view_args", None)
+        self._view_args = getattr(obj, "_view_args", None)
 
     def __setitem__(self, idx: object, value: object) -> None:
         if (ref := self._view_args) is not None and not self._is_element:
@@ -215,18 +215,17 @@ class ArrayView(_SetItemMixin, np.ndarray):
         return self.copy()
 
 
-class _MaskedSubarrayView(_SetItemMixin, np.ndarray):
+class _MaskedSubarrayView(ArrayView):
     _attr: ClassVar[str]
 
-    def __new__(cls, input_array: np.ndarray, view_args: ElementRef | None):
+    def __new__(
+        cls, input_array: np.ndarray, view_args: ElementRef | None, *, is_element: bool
+    ):
         arr = np.asanyarray(input_array).view(cls)
         arr._view_args = view_args
+        arr._is_element = is_element
         return arr
 
-    def __array_finalize__(self, obj: np.ndarray | None):
-        if obj is not None:
-            self._view_args = getattr(obj, "_view_args", None)
-
     def _view_update_target(self, container: _SupportsSetItem) -> _SupportsSetItem:
         return getattr(container, self._attr)
 
@@ -239,7 +238,7 @@ class _DataView(_MaskedSubarrayView):
     _attr = "data"
 
 
-class MaskedArrayView(_SetItemMixin, np.ma.MaskedArray):
+class MaskedArrayView(ArrayView, np.ma.MaskedArray):
     def __new__(
         cls,
         input_array: Sequence[Any],
@@ -250,12 +249,20 @@ class MaskedArrayView(_SetItemMixin, np.ma.MaskedArray):
         if view_args is not None:
             view_args = ElementRef(*view_args)
         arr._view_args = view_args
+        arr._is_element = True
         return arr
 
-    def __array_finalize__(self, obj: np.ndarray | None):
-        super().__array_finalize__(obj)
-        if obj is not None:
-            self._view_args = getattr(obj, "_view_args", None)
+    def _update_from(self, obj) -> None:
+        # `np.ma`’s `_arraymethod` wrappers (`.T`, `.reshape`, …) build the result
+        # from `self._data`, so `__array_finalize__` never sees `self`.
+        super()._update_from(obj)
+        self._view_args = getattr(obj, "_view_args", None)
+        self._is_element = False
+
+    def __reduce__(self):
+        # `np.ma`’s reconstructor calls `cls.__new__(cls, data, mask=…, dtype=…)`,
+        # which our `__new__` can’t accept; pickle as a plain masked array.
+        return np.ma.MaskedArray(self, subok=False, copy=True).__reduce__()
 
     @property
     def mask(self) -> _MaskView | np.bool_:
@@ -264,15 +271,24 @@ class MaskedArrayView(_SetItemMixin, np.ma.MaskedArray):
             # `nomask` sentinel: many numpy.ma internals rely on `is nomask`
             # identity, so don’t wrap it in a view.
             return m
-        return _MaskView(m, self._view_args)
+        return _MaskView(m, self._view_args, is_element=self._is_element)
 
     @mask.setter
     def mask(self, value: ArrayLike) -> None:
-        self.__setmask__(value)  # type: ignore[arg-type]
+        if self._view_args is None:
+            self.__setmask__(value)  # type: ignore[arg-type]
+            return
+        msg = (
+            f"Trying to modify attribute `.{self._view_args.attrname}` of view, "
+            "initializing view as actual."
+        )
+        warn(msg, ImplicitModificationWarning)
+        with view_update(*self._view_args) as container:
+            container.mask = value
 
     @property
     def data(self) -> _DataView:  # type: ignore[override]
-        return _DataView(super().data, self._view_args)
+        return _DataView(super().data, self._view_args, is_element=self._is_element)
 
     def copy(self, *args, **kwargs) -> np.ma.MaskedArray:  # type: ignore[override]
         return np.ma.MaskedArray(self, subok=False, copy=True)
diff --git a/src/anndata/_io/specs/methods.py b/src/anndata/_io/specs/methods.py
index a0c78bcf..e94bf1a9 100644
--- a/src/anndata/_io/specs/methods.py
+++ b/src/anndata/_io/specs/methods.py
@@ -476,9 +476,11 @@ def write_list(
 @_REGISTRY.register_write(h5py.Group, views.ArrayView, IOSpec("array", "0.2.0"))
 @_REGISTRY.register_write(h5py.Group, np.ndarray, IOSpec("array", "0.2.0"))
 @_REGISTRY.register_write(h5py.Group, np.ma.MaskedArray, IOSpec("array", "0.2.0"))
+@_REGISTRY.register_write(h5py.Group, views.MaskedArrayView, IOSpec("array", "0.2.0"))
 @_REGISTRY.register_write(zarr.Group, views.ArrayView, IOSpec("array", "0.2.0"))
 @_REGISTRY.register_write(zarr.Group, np.ndarray, IOSpec("array", "0.2.0"))
 @_REGISTRY.register_write(zarr.Group, np.ma.MaskedArray, IOSpec("array", "0.2.0"))
+@_REGISTRY.register_write(zarr.Group, views.MaskedArrayView, IOSpec("array", "0.2.0"))
 @_REGISTRY.register_write(zarr.Group, zarr.Array, IOSpec("array", "0.2.0"))
 @_REGISTRY.register_write(zarr.Group, h5py.Dataset, IOSpec("array", "0.2.0"))
 @suppress_autoshard_warning

[flying-sheep]

write_h5ad/write_zarr of a view with masked X/layers/obsm

I don’t think that any of the “silently loses mask” behaviors in the existing code can be called desirable, but that’s what we’re doing when writing a non-view MaskedArray (complete with TODO about changing it lol), so I’ll make the writing happen.

pickle(view.obsm["masked"])

Good thing it fails as an unpickled ArrayView is just broken. So adata.copy() is a really good idea if you want to pickle.

ad.concat([a[:5], a[5:]])

Good catch, this one should definitely not maintain the subclass but instead create a regular MaskedArray

What happens when concatenating ArrayViews here? Same bug or do we handle that?

AnnData(obsm={"m": view_el + 1})

I see, so __new__ or whatever should actualize a MaskedArray here.

I wonder what happens when you do this for ArrayView.

write via .T/.reshape/.ravel/.swapaxes/.squeeze/.transpose/.real

that’s what the other PR is supposed to handle for regular ArrayView, I suspected that it’d be handled separately here.