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

The method for annotating genes with cell types #812

Closed
wants to merge 7 commits into from

Conversation

PrimozGodec
Copy link

With this PR we propose our new annotation method to Scannpy:

Annotator marks the data with cell type annotations based on marker genes.
Over-expressed genes are selected with the Mann-Whitney U tests and cell
types are assigned with the hypergeometric test. This function first selects
genes from gene expression data with the Mann Whitney U test, then annotate
them with the hypergeometric test, and finally filter out cell types that
have zero scores for all cells. The results are scores that tell how
probable each cell type is for each cell.

Hope you like the method and merge it to Scampy.

@ivirshup
Copy link
Member

ivirshup commented Sep 5, 2019

Thanks for the PR! Gene-set based annotation would be pretty useful to have here.

Is there any chance there's a preprint we could look at for a little more context on the method?

@PrimozGodec
Copy link
Author

@ivirshup it is currently in writing. Will write back to you when we will have it ready.

@fidelram
Copy link
Collaborator

fidelram commented Oct 1, 2019

Any update on this? Can you add a test (probably reusing the example already in the method docstring)?

@PrimozGodec
Copy link
Author

@fidelram, we are still working paper/preprint. I will post it soon.

I will add tests. So in order for the test to work should I add my library in the requirements.txt? What I observed is that other external packages are not included in project requirements.

@fidelram
Copy link
Collaborator

fidelram commented Oct 2, 2019

Yeah, please add it together with a comment mentioning where is needed (e.g external.tl.annotator )

@ivirshup
Copy link
Member

ivirshup commented Oct 2, 2019

@PrimozGodec, probably don't add this to requirements.txt, since the requirement should be optional for install. I think instead you should mark it with something like:

from importlib.util import find_spec

@pytest.mark.skipif(find_spec('pointannotator') is None, reason="pointannotator not installed")

You can add a requirement for the package to this line in setup.py: https://github.com/theislab/scanpy/blob/d8f32c040f3a5f4fc07998b269796ca58de84b40/setup.py#L41

Maybe we should eventually have a second requirements file for CI testing, like we do for anndata.

@PrimozGodec PrimozGodec force-pushed the annotator branch 9 times, most recently from 14432ce to afe7fa9 Compare October 3, 2019 13:48
@PrimozGodec
Copy link
Author

I added unit tests and reformated the code.

@flying-sheep
Copy link
Member

Thank you. We’re using pytest though, so please write the tests that way:

  1. Remove the class and make all its methods top-level functions
  2. Make setUp into fixtures
  3. Just use assert
@pytest.fixture
def markers():
    return pd.DataFrame(
        ...
    )


@pytest.fixture
def adata():
    ...
    return AnnData(data.values, var=data.columns.values)


def test_remove_empty_column(adata, markers):
    ...
    annotations = annotator(adata, markers, num_genes=20)
    ...
    assert len(annotations) == len(self.anndata)
    ...

@flying-sheep
Copy link
Member

flying-sheep commented Oct 7, 2019

Only remaining thought: I have slight concerns about the name being too generic, but then again, this does exactly what people expect a “cell type annotator based on marker genes” to do.

@Zethson
Copy link
Member

Zethson commented May 8, 2024

We decided not to add more packages to external but you are more than welcome to add your own package to the scverse ecosystem: https://scverse.org/packages/#ecosystem

@Zethson Zethson closed this May 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants