Clean up dataloader, remove drop_last as int
#1975
Conversation
adamgayoso
requested review from
justjhong,
PierreBoyeau,
martinkim0 and
canergen
adamgayoso
changed the title
drop_last as int
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #1975 +/- ##
==========================================
- Coverage 89.14% 89.13% -0.02%
==========================================
Files 141 141
Lines 11113 11079 -34
==========================================
- Hits 9907 9875 -32
+ Misses 1206 1204 -2
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 2 files with indirect coverage changes Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report in Codecov by Sentry. |
| def test_ann_dataloader(): | ||
| a = scvi.data.synthetic_iid() | ||
| @pytest.mark.parametrize( | ||
| "data", [scvi.data.synthetic_iid(200), scvi.data.synthetic_iid(200, sparse=True)] |
There was a problem hiding this comment.
what's simple? pytest parameterize?
| iter_ndarray: bool = False, | ||
| **data_loader_kwargs, | ||
| ): | ||
| if adata_manager.adata is None: |
There was a problem hiding this comment.
surprised that you can delete this w/o any test breaking. it's been too long so I forget why this code is here in the first place
There was a problem hiding this comment.
nvm i can see it was moved to dataset
Fixes #1892
AnnDataLoader(typing, moving checks into AnnTorchDataset)AnnTorchDataset, made some methods privateThe most important change here is removing the feature to have
drop_lastbe an integer. This is something we've had since the beginning of scVI and it was more relevant when datasets were smaller. At this point the feature is more of a pain as it would have to be disabled anyway under some multi gpu training settings where you need even batch sizes. I'm curious what everyone thinks about this breaking change.