-
Notifications
You must be signed in to change notification settings - Fork 2
Add SCVIDataModule
, pytorch_scvi.ipynb
#31
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
base: main
Are you sure you want to change the base?
Conversation
27010b1
to
f20bd58
Compare
* Updating pytorch_scvi notebook to fix UMAP plots * `pytorch_scvi.ipynb`: rm metadata, some warnings * `pytorch_scvi`: update `nnz` comment --------- Co-authored-by: Konstantinos Tsitsimpikos <kostas.t@tiledb.com> Co-authored-by: Ryan Williams <ryan.williams@tiledb.com>
if not inplace: | ||
obs_df = obs_df.copy() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since pandas is moving towards Copy-Over-Write logic soon (defauly behaviour in Pandas3.0 +). We may want to be proactive and remove this inplace
behaviour
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
_add_batch_col(inplace=True)
is used at both call-sites in this file, to avoid an unnecessary copy. The implementation doesn't use any deprecated Pandas kwargs/APIs, and we'll want to support pandas<3
for some years, so I would leave this as-is.
Or: how would you propose rewriting it (without adding an unnecessary copy()
)?
Porting @mlin's
SCVIDataModule
andpytorch_scvi.ipynb
from chanzuckerberg/cellxgene-census#1368.TODO: