refactor: normalize_total with Numba
#3593
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
@flying-sheep ok I see what you wanted but I thought you wanted to squash my merge into Severin's branch then review on that branch. I am on it |
|
well, then do the same with severin’s branch instead of |
selmanozleyen
force-pushed
the
integrated_normalize_update
branch
from
cd7ab7d to
a3e5cf0
Compare
|
ok I did change the base to main in the PR so that there is only two files to review. |
flying-sheep
force-pushed
the
integrated_normalize_update
branch
from
a3e5cf0 to
ce6cf62
Compare
flying-sheep
approved these changes
Apr 15, 2025
Comment on lines
+282
to
+283
| if not inplace: | ||
| x = x.copy() |
There was a problem hiding this comment.
converting a csc to a csr matrix will copy already, but I guess an extra copy in that case isn’t too bad.
(I get that we also need to enter this path for non-csparse x)
Comment on lines
+85
to
+86
|
|
||
| It returns the normalized data, the counts per cell, and the gene subset. |
There was a problem hiding this comment.
Suggested change
| It returns the normalized data, the counts per cell, and the gene subset. |
No need to say that twice
flying-sheep
merged commit b27d218
into
scverse:normalize_total_update
3 of 8 checks passed
ilan-gold
added a commit
that referenced
this pull request
May 19, 2025
* update normalize_total & remove dep * refactor: `normalize_total` with Numba (#3593) Co-authored-by: Philipp A. <flying-sheep@web.de> * fix doctest * add release note * Switch from profimp to tuna for parsing import profiles (#3620) * [pre-commit.ci] pre-commit autoupdate (#3622) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Philipp A. <flying-sheep@web.de> * Fix typo (#3555) Co-authored-by: Phil Schaf <flying-sheep@web.de> * Switch to fast-array-utils (#3598) Co-authored-by: Rodrigo Goya <rgoya@users.noreply.github.com> * tSNE components parameter (#2803) Co-authored-by: Phil Schaf <flying-sheep@web.de> * Simplify scale (#3351) Co-authored-by: Intron7 <severin.dicks@icloud.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> * remove layers test * explicit return and fix the info message * remove unnecessary ravel's * precision fix * (fix): maintain data type properly. * (fix): remove artifact storage * (refactor): use nice f-a-u typing --------- Co-authored-by: Philipp A. <flying-sheep@web.de> Co-authored-by: Selman Özleyen <32667648+selmanozleyen@users.noreply.github.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Swastik Mishra <4453983+swstkm@users.noreply.github.com> Co-authored-by: Rodrigo Goya <rgoya@users.noreply.github.com> Co-authored-by: Kitsune <48340051+ch1ru@users.noreply.github.com> Co-authored-by: selmanozleyen <syozleyen@gmail.com> Co-authored-by: ilan-gold <ilanbassgold@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Hi this PR integrates the recent commits and refactors on #3571.
Here I removed the division part from the numba code so that it is unified in the future with fast-array-utils elem_mult.
pinging bc I can't ask for reviews for some reason: @flying-sheep