Skip to content

Public api change: rename adata to data and make it positional only #1180

@selmanozleyen

Description

@selmanozleyen

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,
    ...
) -> ...:

Originally posted by @flying-sheep in #1134 (comment)

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions