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

Fix notebook bugs and dependency issues #281

Merged
merged 3 commits into from
Mar 31, 2024
Merged

Fix notebook bugs and dependency issues #281

merged 3 commits into from
Mar 31, 2024

Conversation

njmei
Copy link
Contributor

@njmei njmei commented Mar 30, 2024

This PR is partially in response to:
#277

Notes:

  • I've now run all notebooks except docs/tutorials/atlas.ipynb to ensure they work properly.
    • The atlas.ipynb requires a bit too much data to download for me to test
  • Please see individual commit messages for additional details

Questions:

  • It looks like there is overlap in terms of how to install dependencies like scanorama>=1.7.3 and harmonypy>=0.0.9. I see them in extra under [project.optional-dependencies] but would it make more sense to remove them from extra and just keep the ones I newly added under recommend? Or would you prefer the opposite (move packages like magic-impute to extra)?

In the original cell the following code was specified:

```
data = snap.read_dataset(
    "colon.h5ads",
    update_data_locations = {"colon_transverse_SM-CSSDA": "colon_transverse_SM-CSSDA.h5ad"},
)
data
```

This raises the following error:
```
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
Cell In[25], line 1
----> 1 data = snap.read_dataset(
      2     "colon.h5ads",
      3     update_data_locations = {"colon_transverse_SM-CSSDA": "colon_transverse_SM-CSSDA.h5ad"},
      4 )
      5 data

TypeError: read_dataset() got an unexpected keyword argument 'update_data_locations'
```

Looking at the `read_dataset` function definition in pyanndata-0.3.2:
https://docs.rs/pyanndata/0.3.2/pyanndata/anndata/fn.read_dataset.html

It looks like the arg `update_data_locations` should actually be
`adata_files_update`.
While running SnapAtac2 notebooks the following warning was
fairly frequently encountered when running `snap.pp.scrublet()`:

```
/usr/local/lib/python3.11/site-packages/anndata/_core/anndata.py:522: FutureWarning:

The dtype argument is deprecated and will be removed in late 2024.
```

It looks like the `anndata` project plans to deprecate and completely
remove the `dtype` argument to `AnnData` instantiation somewhat soon.
See: scverse/anndata@85162c7
See also: scverse/anndata#1126

This commit removes the `dtype` argument from `AnnData` instantiation
as it will be removed in future versions of `AnnData`.
This commit adds some additional dependencies so that more notebooks
under `docs/tutorials` can run properly when installing with
`pip install snapatac2[recommend]`.

In future work, it may make sense to remove dependencies like
`scanorama` and `harmonypy` from `extra`
`[project.optional-dependencies]` as `scanpy` already shares the
exact same dependencies and it may be easier for future maintenance
for `scanpy` to deal with versioning and pinning.
@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.23%. Comparing base (b73dcfd) to head (ca414ce).

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #281   +/-   ##
=======================================
  Coverage   98.23%   98.23%           
=======================================
  Files           4        4           
  Lines         227      227           
=======================================
  Hits          223      223           
  Misses          4        4           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@kaizhang kaizhang merged commit 3bcb4a2 into kaizhang:main Mar 31, 2024
4 checks passed
@kaizhang
Copy link
Owner

Thank you for the pull request! I'll address the extra package issue.

kaizhang pushed a commit that referenced this pull request Apr 1, 2024
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