chore: fix spatialdata access in squidpy via a helper function

[selmanozleyen]

Fixes #1133 and implements the table extraction in a decorator so we won't need to ctrl+f our way to fix a similar bug when it happens

update: decided not to use a decorator as it uses metaprogramming with little to no benefit over using a function. Only plus was not adding table_key to the signature which isn't a plus imo. Explicitly typing a signature of a function is not something I consider as duplicate code

[codecov]

Codecov Report

❌ Patch coverage is 83.33333% with 12 lines in your changes missing coverage. Please review.
✅ Project coverage is 74.20%. Comparing base (093217d) to head (0518d51).

Files with missing lines	Patch %	Lines
src/squidpy/gr/_build.py	79.41%	4 Missing and 3 partials ⚠️
src/squidpy/gr/_niche.py	37.50%	4 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1134      +/-   ##
==========================================
+ Coverage   73.82%   74.20%   +0.38%     
==========================================
  Files          45       45              
  Lines        7013     7016       +3     
  Branches     1188     1179       -9     
==========================================
+ Hits         5177     5206      +29     
+ Misses       1349     1332      -17     
+ Partials      487      478       -9     

Files with missing lines	Coverage Δ
src/squidpy/_constants/_pkg_constants.py	94.20% <100.00%> (+0.30%)	⬆️
src/squidpy/_docs.py	94.28% <100.00%> (+0.08%)	⬆️
src/squidpy/gr/_ligrec.py	78.18% <100.00%> (+0.59%)	⬆️
src/squidpy/gr/_nhood.py	81.48% <100.00%> (+3.29%)	⬆️
src/squidpy/gr/_ppatterns.py	80.43% <100.00%> (+1.55%)	⬆️
src/squidpy/gr/_ripley.py	96.49% <100.00%> (+1.70%)	⬆️
src/squidpy/gr/_sepal.py	56.20% <100.00%> (+1.13%)	⬆️
src/squidpy/gr/_utils.py	69.23% <100.00%> (+2.91%)	⬆️
src/squidpy/tl/_sliding_window.py	77.90% <100.00%> (+2.04%)	⬆️
src/squidpy/gr/_niche.py	37.53% <37.50%> (+0.99%)	⬆️
... and 1 more

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[timtreis]

This is no longer convention in SpatialData due to inherently multi-table techs like VisiumHD or CosMx 🤔 But it might be good to just let the user run into the error msg that tells them about their options if they don't specify it so I think we can leave it like this

[flying-sheep]

isn’t using "table" as default misleading then?

[timtreis]

"adata is sdata" is weird? Maybe it should be resolve_data/input?

[selmanozleyen]

yes but the error message would still have to be the same since the public signature takes adata, otherwise the error message would be confusing

[selmanozleyen]

this is done in many places lets make an issue about it and move on for here. Ideally we would call it data since it's positional argument anyway

[flying-sheep]

Looks great apart from the table issue: #1134 (comment)

I think since there’s no default anymore, it might make sense to require specifying table=... when adata is a SpatialData (and raise a TypeError otherwise, as Python does when a required parameter is missing)

[selmanozleyen]

Thank you @flying-sheep ! Also what do you think of changing the parameter name from adata to data? Since it's a positional argument would the change be smooth?

[flying-sheep]

Makes sense to me! Make sure to add a / to the parameter specification to enforce that people aren’t actually using the name.

And put the / after all the “main” parameters if there are more than just data. Depends on your judgement what a “main” parameters are – it’s often the same list as the mandatory parameters, unless a function has too many mandatory parameters, or multiple mandatory parameters have the same type and the order could be confusing.

I think most good APIs have 1–2 positional-only parameters (might have defaults), and some keyword-only parameters that all have defaults. Most functions have zero positional-or-keyword parameters, but for some it makes sense.

e.g. var_by_distance would look like this:

def var_by_distance(
    adata: AnnData,
    groups: str | list[str] | NDArrayA,
    /,
    *,
    cluster_key: str | None = None,
    ...
) -> ...:

whereas nhood_enrichment would look like this, so people can call it either as nhood_enrichment(adata, "k") or nhood_enrichment(adata, cluster_key=..., library_key=...) to clarify which key is which.

def nhood_enrichment(
    data: AnnData | SpatialData,
    /,
    cluster_key: str,
    *,
    library_key: str | None = None,
    connectivity_key: str | None = None,
    ...
) -> ...: