Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow selection of layer/ raw for methods that use X #828

Open
ivirshup opened this issue Sep 11, 2019 · 2 comments
Open

Allow selection of layer/ raw for methods that use X #828

ivirshup opened this issue Sep 11, 2019 · 2 comments

Comments

@ivirshup
Copy link
Member

I think this could use a consolidated effort for consistent behavior. Especially since testing whether it works will probably have some common patterns. In some cases Raw will need to be an option.

I like the convention of having the arguments use_raw, layers, and (when appropriate) obsm_key/ varm_key. With these at most one of the values can be not None, and if all are None (the default) X is used.

An alternative convention is use_rep: Optional[str]. I’m less a fan of this due to potential key collisions.

Some relevant issues/ prs: #826 #801 #730

@ivirshup
Copy link
Member Author

ivirshup commented Dec 1, 2019

As an update, I've been using this helper function to consistently handle this:

def _choose_obs_rep(adata, *, use_raw=False, layer=None, obsm=None, obsp=None):
    """
    Choose array aligned with obs annotation.
    """
    is_layer = layer is not None
    is_raw = use_raw is not False
    is_obsm = obsm is not None
    is_obsp = obsp is not None
    choices_made = sum((is_layer, is_raw, is_obsm, is_obsp))
    assert choices_made <= 1
    if choices_made == 0:
        return adata.X
    elif is_layer:
        return adata.layers[layer]
    elif use_raw:
        return adata.raw.X
    elif is_obsm:
        return adata.obsm[obsm]
    elif is_obsp:
        return adata.obsp[obsp]
    else:
        assert False, (
            "That was unexpected. Please report this bug at:\n\n\t"
            " https://github.com/theislab/scanpy/issues"
        )

This could use support for variable masks like use_highly_variable. Also the error message should be better.

I think a collection of helper functions like this should go in to a utils module (sc.utils.argutils?) which could be public so it's easier to use in scanpy-like packages.

@ivirshup ivirshup unpinned this issue Feb 25, 2021
@ivirshup
Copy link
Member Author

ivirshup commented Aug 3, 2023

Support for pca and regress_out started in: #2588

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

1 participant