[flying-sheep]

This and the two PRs below it are I need in 0.14 (for scanpy 2.0 preview), except for .uns accessors.

See also zarr-developers/zarr-specs#128

• Closes Allow any unicode incl. / and \ in obsm keys, obs columns, uns keys, … #2450
• Tests added

• Release note not necessary because:

[codecov]

Codecov Report

❌ Patch coverage is 96.82540% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 86.25%. Comparing base (5720f48) to head (167e1f7).
✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
src/anndata/_io/specs/lazy_methods.py	90.90%	2 Missing ⚠️
src/anndata/_io/specs/methods.py	95.23%	2 Missing ⚠️
src/anndata/_io/utils.py	97.75%	2 Missing ⚠️

Additional details and impacted files

@@               Coverage Diff                @@
##           feat/json-io    #2662      +/-   ##
================================================
+ Coverage         85.94%   86.25%   +0.31%     
================================================
  Files                51       51              
  Lines              8390     8508     +118     
================================================
+ Hits               7211     7339     +128     
+ Misses             1179     1169      -10     

Files with missing lines	Coverage Δ
src/anndata/_io/h5ad.py	92.88% <100.00%> (ø)
src/anndata/_io/specs/registry.py	94.82% <100.00%> (+0.13%)	⬆️
src/anndata/_io/zarr.py	87.21% <100.00%> (ø)
src/anndata/_write_compat.py	96.66% <100.00%> (+0.11%)	⬆️
src/anndata/experimental/backed/_io.py	93.44% <100.00%> (+0.10%)	⬆️
src/anndata/experimental/merge.py	90.16% <100.00%> (+1.78%)	⬆️
src/anndata/_io/specs/lazy_methods.py	95.83% <90.90%> (-0.78%)	⬇️
src/anndata/_io/specs/methods.py	93.34% <95.23%> (+1.03%)	⬆️
src/anndata/_io/utils.py	84.37% <97.75%> (+5.52%)	⬆️

... and 1 file with indirect coverage changes

[ilan-gold]

Discussion is a bit mangled now, but scverse/zarr-convention-dataframe#1 is quite a strong suggestion that seems simpler to implement. Maybe let's discuss there and then I will implement there (and you here) whatever mechanism we settle on (your implementation here of other data frame things does not have to match mine otherwise)

[read-the-docs-business]

Documentation build overview

📚 anndata | 🛠️ Build #4678861 | 📁 Comparing 167e1f7 against latest (c4f5138)

  🔍 Preview build  

158 files changed · + 10 added · ± 147 modified · - 1 deleted

+ Added

• citation.html
• installation.html
• write-compat.html
• generated/anndata.AnnData.__contains__.html
• generated/anndata.AnnData.__getitem__.html
• generated/anndata.AnnData.__sizeof__.html
• generated/anndata.WriteCompat.html
• generated/anndata.WriteConfig.html
• generated/anndata.experimental.AnnCollection.__getitem__.html
• generated/anndata.types.WriteCompatStr.html

± Modified

• _key_contributors.html
• accessors.html
• api.html
• contributing.html
• fileformat-prose.html
• genindex.html
• index.html
• interoperability.html
• py-modindex.html
• references.html
• and 137 more...

- Deleted

• _static/sbt-webpack-macros.html

[selmanozleyen]

compat needs to be passed through write_use.

Otherwise these fail:

@pytest.mark.parametrize(
    "existing", ["x", "x/y"], ids=["unescaped-group", "escaped-group"]
)
@pytest.mark.parametrize("key", ["a/b", "100%"])
@pytest.mark.parametrize(
    "use", [lambda k: A.obsm[k], lambda _: A.obsm], ids=["single-key", "mapping"]
)
def test_use_escapes_keys(
    diskfmt_store: Path | MemoryStore,
    diskfmt: Literal["h5ad", "zarr"],
    existing: str,
    key: str,
    use: Callable[[str], object],
) -> None:
    """`use=` escapes keys like a full write with the same `compat` does.

    `100%` into an already escaped group is the silent case:
    written literally, it would be read back as an escape sequence.
    """
    adata = ad.AnnData(np.zeros((2, 3)), obsm={existing: np.zeros((2, 2))})
    getattr(adata, f"write_{diskfmt}")(diskfmt_store, compat="0.14")

    adata.obsm[key] = np.ones((2, 2))
    getattr(adata, f"write_{diskfmt}")(diskfmt_store, use=use(key), compat="0.14")
    back = getattr(ad, f"read_{diskfmt}")(diskfmt_store)

    assert set(back.obsm) == {existing, key}
    np.testing.assert_array_equal(back.obsm[key], np.ones((2, 2)))

[ilan-gold]

anndata/src/anndata/_settings.py

Line 25 in c8bbe5a

disallow_forward_slash_in_h5ad: bool = True

should be removed wholesale (and maybe a deprecation cycle kicked off)

Unrelated, going to suggest taking a page out of Davis' Bennet's book, and having Claude do an adversarial test of this. Can Claude come up with use-cases where this breaks?

[flying-sheep]

OK, did the adversarial review, it found the following issues with this PR:

• read_lazy returns the escaped keys
• concat_on_disk merges escaped and unescaped keys because it operates at the raw storage level
• unescape_key doesn’t error on all malformed keys
• passing the wrong type as key (e.g. int) now has a worse message

It also found these, which I’d say we ignore:

• zarr v2 reserved names aren’t covered

It also flagged these issues that aren’t ours:

• Zarr can’t deal with quirks like windows path character limit, case-insensitive filenames, and the old MacOS HFS+ file system’s quirks – I say we gotta draw the line about what we want to mitigate

  I gotta say though that this is the first big HDF5 win compared to zarr – not using file names just avoids a bait shop’s whole inventory of cans of worms

  That does make me think though – maybe we should go back on the escaping idea and instead use fixed key and attribute names (like in the data frame convention) as it avoids all these issues.

• fsspec has a bad bug that affects the escaping scheme: Inconsistencies regarding quoting / escaping in URLs / paths fsspec/filesystem_spec#1713

  We should address that ASAP before merging. I don’t want to use a different escape scheme just because of that as it’s a horrible bug for zarr-over HTTP that can’t remain unfixed

[flying-sheep]

OK changed the architecture:

• does both dicts and data frames again
• now uses the zarr convention (except on HDF5) and references it in the docs
• uses a variant of the convention with less dataframe-specific names for dicts (keys and item_{n})
• allows missing indices (anndata doesn’t write Dataset2D to .obsm so that isn’t exercised yet)
• allows anonymous indices (the convention doesn’t address this, I decided to just use "" for that. I feel like the 3 people who name indices at all won’t care if an index is empty-named or unnamed)