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

Update filter x #324

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Update filter x #324

wants to merge 3 commits into from

Conversation

Intron7
Copy link
Member

@Intron7 Intron7 commented Feb 6, 2025

No description provided.

@Intron7 Intron7 requested a review from flying-sheep February 6, 2025 16:23
@Intron7 Intron7 added the run-gpu-ci runs GPU CI label Feb 6, 2025
@github-actions github-actions bot removed the run-gpu-ci runs GPU CI label Feb 6, 2025
@Intron7 Intron7 requested review from ilan-gold and removed request for flying-sheep February 7, 2025 12:19
@ilan-gold
Copy link
Contributor

Could you highlight what you changed? Did some behavior change?

@Intron7
Copy link
Member Author

Intron7 commented Feb 11, 2025

@ilan-gold so basically before I did something slightly different than what scanpy does in those functions. Now they do exactly the same thing. I had to pass a qc_var and than I took the value from obs or var and did not recalculate the values

@ilan-gold
Copy link
Contributor

ilan-gold commented Feb 12, 2025

but have the results changed? like do you still filter out the same genes/cells for the "same" arguments? that's not clear to me

@Intron7
Copy link
Member Author

Intron7 commented Feb 12, 2025

@ilan-gold The results have not changed but the syntax has. So its a breaking change.

Comment on lines -57 to +64
qc_var: str = "n_cells_by_counts",
min_count: int = None,
max_count: int = None,
min_counts: int | None = None,
min_cells: int | None = None,
max_counts: int | None = None,
max_cells: int | None = None,
inplace: bool = True,
Copy link
Member

Choose a reason for hiding this comment

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

@ilan-gold these API changes are what this PR is about.

Copy link
Member

@flying-sheep flying-sheep left a comment

Choose a reason for hiding this comment

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

Looks good to me!

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.

3 participants