Closed pcm32 closed 8 months ago
Since mnnpy is not taking in the PRs and this is fixed in conda, we should move testing to conda entirely.
@pmb59 @anilthanki I would like to give you the chance to review this if you consider it relevant (atlas sc pipelines rely on this package, although of course all the atlas usages are pinned in Galaxy, so this doesn't affect currently running setup). If I don't hear from you guys in a couple of days I'll just merge, as you can see tests are passing.
There is only minor functionality changes on this PR, mostly move to latest scanpy, CI revamp to rely on mamba and resolve some dependency pinnings (bknn, mnn, scipy).
Thanks for the review @pmb59 !
Addresses #123
It would be good to re-run tests here to make sure versions still work well and then merge and release.
For the record, we stopped supporting mmnpy due to this testing error:
not ok 43 Run MNN batch integration using clustering as batch
# (in test file scanpy-scripts-tests.bats, line 662)
# `run rm -f $mnn_obj && eval "$scanpy integrate mnn $mnn_opt $louvain_obj $mnn_obj"' failed with status 132
# /home/runner/micromamba-root/envs/scanpy-scripts/lib/python3.9/site-packages/anndata/__init__.py:51: FutureWarning: `anndata.read` is deprecated, use `anndata.read_h5ad` instead. `ad.read` will be removed in mid 2024.
# warnings.warn(
# /home/runner/micromamba-root/envs/scanpy-scripts/lib/python3.9/site-packages/mnnpy/utils.py:20: NumbaDeprecationWarning: The 'nopython' keyword argument was not supplied to the 'numba.jit' decorator. The implicit default value for this argument is currently False, but it will be changed to True in Numba 0.59.0. See https://numba.readthedocs.io/en/stable/reference/deprecation.html#deprecation-of-object-mode-fall-back-behaviour-when-using-jit for details.
# @jit(float32[:, :](float32[:, :], float32[:, :]), nogil=True)
# /home/runner/micromamba-root/envs/scanpy-scripts/lib/python3.9/site-packages/mnnpy/utils.py:25: NumbaDeprecationWarning: The 'nopython' keyword argument was not supplied to the 'numba.jit' decorator. The implicit default value for this argument is currently False, but it will be changed to True in Numba 0.59.0. See https://numba.readthedocs.io/en/stable/reference/deprecation.html#deprecation-of-object-mode-fall-back-behaviour-when-using-jit for details.
# @jit(float32[:, :](float32[:, :], float32[:, :]))
# /home/runner/micromamba-root/envs/scanpy-scripts/lib/python3.9/site-packages/mnnpy/utils.py:30: NumbaPerformanceWarning: np.dot() is faster on contiguous arrays, called on (Array(float32, 1, 'A', False, aligned=True), Array(float32, 1, 'A', False, aligned=True))
# dist[i, j] = np.dot(m[i], n[j])
# 24-01-21 10:13:08; INFO; transforms.py; _extract_loop_lifting_candidates(): finding looplift candidates
# 24-01-21 10:13:09; INFO; transforms.py; _extract_loop_lifting_candidates(): finding looplift candidates
# /home/runner/micromamba-root/envs/scanpy-scripts/lib/python3.9/site-packages/mnnpy/utils.py:205: NumbaPerformanceWarning: np.dot() is faster on contiguous arrays, called on (Array(float32, 1, 'C', False, aligned=True), Array(float32, 1, 'A', False, aligned=True))
# scale = np.dot(working, grad)
# /home/runner/micromamba-root/envs/scanpy-scripts/lib/python3.9/site-packages/mnnpy/utils.py:210: NumbaDeprecationWarning: The 'nopython' keyword argument was not supplied to the 'numba.jit' decorator. The implicit default value for this argument is currently False, but it will be changed to True in Numba 0.[59](https://github.com/ebi-gene-expression-group/scanpy-scripts/actions/runs/7600509637/job/20698846493#step:6:60).0. See https://numba.readthedocs.io/en/stable/reference/deprecation.html#deprecation-of-object-mode-fall-back-behaviour-when-using-jit for details.
# @jit(float32(float32[:, :], float32[:, :], float32[:], float32[:], float32), nogil=True)
# /home/runner/micromamba-root/envs/scanpy-scripts/lib/python3.9/site-packages/mnnpy/utils.py:215: NumbaPerformanceWarning: np.dot() is faster on contiguous arrays, called on (Array(float32, 1, 'C', False, aligned=True), Array(float32, 1, 'A', False, aligned=True))
# curproj = np.dot(grad, curcell)
# /tmp/bats.31[62](https://github.com/ebi-gene-expression-group/scanpy-scripts/actions/runs/7600509637/job/20698846493#step:6:63).src: line 662: 17[64](https://github.com/ebi-gene-expression-group/scanpy-scripts/actions/runs/7600509637/job/20698846493#step:6:65)4 Illegal instruction (core dumped) scanpy-cli integrate mnn --save-layer uncorrected --batch-key louvain_k10_r0_5 post_install_tests/outputs/louvain.h5ad post_install_tests/outputs/mnn.h5ad
I leave this here since test logs gets lost after sometime. If anyone know how to fix this, we would be interested in knowing :-).
Given that we are not going to support mmnpy, I would suggest as well removing the extra pinning tried for supporting it. I would leave the dependencies part like this https://github.com/ebi-gene-expression-group/scanpy-scripts/blob/5f00501bb0984af55cda04fafb5fdbd3e539d51d/test-env.yaml as it has the minimal pinning to make all the rest work.
If you look at the 5f00 commit tests, all other tests are passing and it is right before the pinning changes for mnnpy that never worked.
Great, thanks for finishing this off @anilthanki !