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

Add sparsificiation step before sparse-dependent Scrublet calls #1707

Merged
merged 3 commits into from
Mar 3, 2021

Conversation

pinin4fjords
Copy link
Contributor

This PR addresses #1645, which was caused by my injection of normalised matrices from Scanpy workflows, thereby bypassing a sparseness check Scrublet does with the raw matrix.

The fix contained here is apply the Scrublet sparseness check, before calling Scrublet functions with a dependency on sparseness.

Copy link
Member

@ivirshup ivirshup left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the fix! And test!


Minor point on import, typically scipy.sparse isn't a way you can access that attribute:

In [1]: import scipy

In [2]: scipy.sparse
---------------------------------------------------------------------------
AttributeError                            Traceback (most recent call last)
<ipython-input-2-f25716665fba> in <module>
----> 1 scipy.sparse

AttributeError: module 'scipy' has no attribute 'sparse'

I think it works here because of the order of imports (anndata import scipy.sparse, so it becomes visible), but that may change in the future. Could you import the sparse module directly instead?

from scipy import sparse

scanpy/external/pp/_scrublet.py Outdated Show resolved Hide resolved
pinin4fjords and others added 2 commits March 3, 2021 09:04
Co-authored-by: Isaac Virshup <ivirshup@gmail.com>
@ivirshup ivirshup added this to the 1.7.2 milestone Mar 3, 2021
@ivirshup ivirshup linked an issue Mar 3, 2021 that may be closed by this pull request
3 tasks
@ivirshup ivirshup self-requested a review March 3, 2021 10:05
Copy link
Member

@ivirshup ivirshup left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great, thanks!

@ivirshup ivirshup merged commit 9fa0102 into scverse:master Mar 3, 2021
@ivirshup
Copy link
Member

ivirshup commented Mar 3, 2021

@meeseeksdev backport to 1.7.x

meeseeksmachine pushed a commit to meeseeksmachine/scanpy that referenced this pull request Mar 3, 2021
ivirshup pushed a commit that referenced this pull request Mar 3, 2021
…crublet calls (#1711)

Co-authored-by: Jonathan Manning <jmanning@ebi.ac.uk>
Zethson pushed a commit that referenced this pull request Mar 15, 2021
* Add sparsificiation step before sparse-dependent Scrublet calls

* Apply sparsification suggestion

Co-authored-by: Isaac Virshup <ivirshup@gmail.com>

* Fix imports

Co-authored-by: Isaac Virshup <ivirshup@gmail.com>
ivirshup added a commit that referenced this pull request Mar 18, 2021
* add flake8 pre-commit

Signed-off-by: Zethson <lukas.heumos@posteo.net>

* fix pre-commit

Signed-off-by: Zethson <lukas.heumos@posteo.net>

* add E402 to flake8 ignore

Signed-off-by: Zethson <lukas.heumos@posteo.net>

* revert neighbors

Signed-off-by: Zethson <lukas.heumos@posteo.net>

* fix flake8

Signed-off-by: Zethson <lukas.heumos@posteo.net>

* address review

Signed-off-by: Zethson <lukas.heumos@posteo.net>

* fix comment character in .flake8

Signed-off-by: Zethson <lukas.heumos@posteo.net>

* fix test

Signed-off-by: Zethson <lukas.heumos@posteo.net>

* black

Signed-off-by: Zethson <lukas.heumos@posteo.net>

* review round 2

Signed-off-by: Zethson <lukas.heumos@posteo.net>

* review round 3

Signed-off-by: Zethson <lukas.heumos@posteo.net>

* readded double comments

Signed-off-by: Zethson <lukas.heumos@posteo.net>

* Ignoring E262 & reverted comment

Signed-off-by: Zethson <lukas.heumos@posteo.net>

* using self for obs_tidy

Signed-off-by: Zethson <lukas.heumos@posteo.net>

* Restore setup.py

* rm call of black test (#1690)

* Fix print_versions for python<3.8 (#1691)

* add codecov so we can have a badge to point to (#1693)

* Attempt server-side search (#1672)

* Fix paga_path (#1047)

Fix paga_path

Co-authored-by: Isaac Virshup <ivirshup@gmail.com>

* Switch to flit

This reverts commit d645790

* add setup.py while leaving it ignored

* Update install instructions

* Circumvent new pip check (see pypa/pip#9628)

* Go back to regular pip (#1702)

* Go back to regular flit

Co-authored-by: Isaac Virshup <ivirshup@gmail.com>

* codecov comment (#1704)

* Use joblib for parallelism in regress_out (#1695)

* Use joblib for parallism in regress_out

* release note

* fix link in release notes

* Add todo for resource test

* Add sparsificiation step before sparse-dependent Scrublet calls (#1707)

* Add sparsificiation step before sparse-dependent Scrublet calls

* Apply sparsification suggestion

Co-authored-by: Isaac Virshup <ivirshup@gmail.com>

* Fix imports

Co-authored-by: Isaac Virshup <ivirshup@gmail.com>

* Fix version on Travis (#1713)

By default, Travis does `git clone --depth=50` which means the version can’t be detected from the git tag.

* `sc.metrics` module (add confusion matrix & Geary's C methods) (#915)

* Add `sc.metrics` with `gearys_c`

Add a module for computing useful metrics. Started off with Geary's C since I'm using it and finding it useful. I've also got a fairly fast way to calculate it worked out.

Unfortunatly my implementation runs into some issues with some global configs set by umap (see lmcinnes/umap#306), so I'm going to see if that can be resolved before changing it.

* Add sc.metrics.confusion_matrix

* Better tests and output for confusion_matrix

* Workaround umap<0.4 and increase numerical stability of gearys_c

* Work around lmcinnes/umap#306 by not
  calling out to kernel function. That code has been kept, but commented
  out.
* Increase numerical stability by casting data to system width. Tests
  were failing due to instability.

* Split up gearys_c tests

* Improved unexpected error message

* gearys_c working again. Sadly, a bit slower

* One option for doc strings

* Simplify implementation to use single dispatch

* release notes

* Fix clipped images in docs (#1717)

* Cleanup normalize_total (#1667)

* Cleanup normalize_total

* Add modification tests and copy kwarg for normalize_total

* Test that 'layers' argument is deprecated

* Added more mutation checks for normalize_total

* release note

* Error message

* deprecate scvi (#1703)

* deprecate scvi

* Update .azure-pipelines.yml

Co-authored-by: Isaac Virshup <ivirshup@gmail.com>

* remove :func: links to scvi in release notes

* remove tildes in front of scvi in release notes

* Update docs/release-notes/1.5.0.rst

Co-authored-by: Michael Jayasuriya <mjayasur@Michaels-MacBook-Air.local>
Co-authored-by: Isaac Virshup <ivirshup@gmail.com>

* updated ecosystem.rst to add triku (#1722)

* Minor addition to contributing docs (#1726)

* Preserve category order when groupby is a list (#1735)

Preserve category order when groupby is a list

* Asymmetrical diverging colormaps and vcenter (#1551)

Add vcenter and norm arguments to plotting functions

* add flake8 pre-commit

Signed-off-by: Zethson <lukas.heumos@posteo.net>

* add E402 to flake8 ignore

Signed-off-by: Zethson <lukas.heumos@posteo.net>

* revert neighbors

Signed-off-by: Zethson <lukas.heumos@posteo.net>

* address review

Signed-off-by: Zethson <lukas.heumos@posteo.net>

* black

Signed-off-by: Zethson <lukas.heumos@posteo.net>

* using self for obs_tidy

Signed-off-by: Zethson <lukas.heumos@posteo.net>

* rebased

Signed-off-by: Zethson <lukas.heumos@posteo.net>

* rebasing

Signed-off-by: Zethson <lukas.heumos@posteo.net>

* rebasing

Signed-off-by: Zethson <lukas.heumos@posteo.net>

* rebasing

Signed-off-by: Zethson <lukas.heumos@posteo.net>

* add flake8 to dev docs

Signed-off-by: Zethson <lukas.heumos@posteo.net>

* add autopep8 to pre-commits

Signed-off-by: Zethson <lukas.heumos@posteo.net>

* add flake8 ignore docs

Signed-off-by: Zethson <lukas.heumos@posteo.net>

* add exception todos

Signed-off-by: Zethson <lukas.heumos@posteo.net>

* add ignore directories

Signed-off-by: Zethson <lukas.heumos@posteo.net>

* reinstated lambdas

Signed-off-by: Zethson <lukas.heumos@posteo.net>

* fix tests

Signed-off-by: Zethson <lukas.heumos@posteo.net>

* fix tests

Signed-off-by: Zethson <lukas.heumos@posteo.net>

* fix tests

Signed-off-by: Zethson <lukas.heumos@posteo.net>

* fix tests

Signed-off-by: Zethson <lukas.heumos@posteo.net>

* fix tests

Signed-off-by: Zethson <lukas.heumos@posteo.net>

* Add E741 to allowed flake8 violations.

Co-authored-by: Isaac Virshup <ivirshup@gmail.com>

* Add F811 flake8 ignore for tests

Co-authored-by: Isaac Virshup <ivirshup@gmail.com>

* Fix mask comparison

Co-authored-by: Isaac Virshup <ivirshup@gmail.com>

* Fix mask comparison

Co-authored-by: Isaac Virshup <ivirshup@gmail.com>

* fix flake8 config file

Signed-off-by: Zethson <lukas.heumos@posteo.net>

* readded autopep8

Signed-off-by: Zethson <lukas.heumos@posteo.net>

* import Literal

Signed-off-by: Zethson <lukas.heumos@posteo.net>

* revert literal import

Signed-off-by: Zethson <lukas.heumos@posteo.net>

* fix scatterplot pca import

Signed-off-by: Zethson <lukas.heumos@posteo.net>

* false comparison & unused vars

Signed-off-by: Zethson <lukas.heumos@posteo.net>

* Add cleaner level determination

Co-authored-by: Isaac Virshup <ivirshup@gmail.com>

* Fix comment formatting

Co-authored-by: Isaac Virshup <ivirshup@gmail.com>

* Add smoother dev documentation

Co-authored-by: Isaac Virshup <ivirshup@gmail.com>

* fix flake8

Signed-off-by: Zethson <lukas.heumos@posteo.net>

* Readd long comment

Co-authored-by: Isaac Virshup <ivirshup@gmail.com>

* Assuming X as array like

Co-authored-by: Isaac Virshup <ivirshup@gmail.com>

* fix flake8

Signed-off-by: Zethson <lukas.heumos@posteo.net>

* fix flake8 config

Signed-off-by: Zethson <lukas.heumos@posteo.net>

* reverted rank_genes

Signed-off-by: Zethson <lukas.heumos@posteo.net>

* fix disp_mean_bin formatting

Co-authored-by: Isaac Virshup <ivirshup@gmail.com>

* fix formatting

Signed-off-by: Zethson <lukas.heumos@posteo.net>

* add final todos

Signed-off-by: Zethson <lukas.heumos@posteo.net>

* boolean checks with is

Signed-off-by: Zethson <lukas.heumos@posteo.net>

* _dpt formatting

Signed-off-by: Zethson <lukas.heumos@posteo.net>

* literal fixes

Signed-off-by: Zethson <lukas.heumos@posteo.net>

* links to leafs

Signed-off-by: Zethson <lukas.heumos@posteo.net>

* revert paga variable naming

Co-authored-by: Philipp A <flying-sheep@web.de>
Co-authored-by: Sergei Rybakov <koncopd@gmail.com>
Co-authored-by: Isaac Virshup <ivirshup@gmail.com>
Co-authored-by: Jonathan Manning <jmanning@ebi.ac.uk>
Co-authored-by: mjayasur <mjayasur@berkeley.edu>
Co-authored-by: Michael Jayasuriya <mjayasur@Michaels-MacBook-Air.local>
Co-authored-by: Alex M. Ascensión <35657291+alexmascension@users.noreply.github.com>
Co-authored-by: Gökçen Eraslan <gokcen.eraslan@gmail.com>
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.

scrublet crashes on dense input
2 participants