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.

Sign up for GitHub

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

Jump to bottom

Skip COO matrix tests for newer anndata #3442

Merged
merged 3 commits into from
Jan 21, 2025
Merged

Skip COO matrix tests for newer anndata #3442

merged 3 commits into from
Jan 21, 2025

Conversation

flying-sheep
Copy link
Member

@flying-sheep flying-sheep commented Jan 17, 2025 •
edited
Loading

  • Release notes not necessary because: test-only fix

@ilan-gold there are more COO tests in scanpy that don’t get triggered.

Did scverse/anndata#1829 disallow CS* matrices in all the spots it should have?

@flying-sheep flying-sheep marked this pull request as ready for review January 17, 2025 13:27
@flying-sheep flying-sheep added this to the 1.11.0 milestone Jan 17, 2025
@ilan-gold
Copy link
Contributor

Did scverse/anndata#1829 disallow CS* matrices in all the spots it should have?

It only disabled declaring the object (or it should have). I intentionally kept it limited in scope. So I didn't look into other ops like setting

@ilan-gold
Copy link
Contributor

On a quick first pass, the tests in the scanpy repo that reference coo besides the one in this PR do not actually interact with the AnnData object.

there are more COO tests in scanpy that don’t get triggered.

Was this referring to something specific?

@codecov Codecov
Copy link

codecov bot commented Jan 17, 2025 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 75.45%. Comparing base (75c246d) to head (2f551e0).
Report is 2 commits behind head on main.

Additional details and impacted files 
@@           Coverage Diff           @@
##             main    #3442   +/-   ##
=======================================
  Coverage   75.45%   75.45%           
=======================================
  Files         113      113           
  Lines       13243    13243           
=======================================
  Hits         9992     9992           
  Misses       3251     3251

@flying-sheep
Copy link
Member Author

nothing specific!

On a quick first pass, the tests in the scanpy repo that reference coo besides the one in this PR do not actually interact with the AnnData object.

huh, I must have looked wrong then.

indeed, only tests for utils and the like, sorry for making you discover that!

ilan-gold
ilan-gold reviewed Jan 20, 2025
View reviewed changes
tests/test_qc_metrics.py Outdated Show resolved Hide resolved
@ilan-gold
Copy link
Contributor

sorry for making you discover that!

No worries, it always good to know.

ilan-gold
ilan-gold reviewed Jan 20, 2025
View reviewed changes
tests/test_qc_metrics.py Outdated Show resolved Hide resolved
@flying-sheep flying-sheep enabled auto-merge (squash) January 21, 2025 08:54
@flying-sheep flying-sheep merged commit 6c89e1d into main Jan 21, 2025
12 checks passed
@flying-sheep flying-sheep deleted the pa/fix-coo branch January 21, 2025 09:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ilan-gold ilan-gold ilan-gold approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
1.11.0
Development

Successfully merging this pull request may close these issues.

Integration Testing CI Test Failure on python 3.10 Integration Testing CI Test Failure on python 3.12
2 participants
@flying-sheep @ilan-gold