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

replace statsmodels with scipy stats #2762

Merged
merged 7 commits into from
May 1, 2024
Merged

replace statsmodels with scipy stats #2762

merged 7 commits into from
May 1, 2024

Conversation

martinkim0
Copy link
Contributor

No description provided.

@martinkim0
Copy link
Contributor Author

Tests are currently failing because pvalue has NaNs (before false_discovery_control is called). This wasn't being caught before for some reason (does statsmodels not check for NaNs?)

@justjhong
Copy link
Contributor

@martinkim0 good catch, looks like statsmodels would just output nans. This happens when the input to chi2.cdf is 0 (which usually shouldn't happen - happens here since its barely training I assume), but we can still correctly handle this case by making the pval 1 for these examples.

@martinkim0
Copy link
Contributor Author

Awesome, thanks for pushing the changes

Copy link

codecov bot commented May 1, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 89.14%. Comparing base (18b4432) to head (7256aca).

Additional details and impacted files
@@           Coverage Diff           @@
##             mrvi    #2762   +/-   ##
=======================================
  Coverage   89.14%   89.14%           
=======================================
  Files         167      167           
  Lines       14232    14232           
=======================================
  Hits        12687    12687           
  Misses       1545     1545           
Files Coverage Δ
src/scvi/external/mrvi/_model.py 90.72% <100.00%> (ø)

@martinkim0 martinkim0 merged commit 87249e2 into mrvi May 1, 2024
6 checks passed
@martinkim0 martinkim0 deleted the mrvi-statsmodels branch May 1, 2024 18:16
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

4 participants