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

Unit tests fail locally after pulling latest changes #656

Closed
WeilerP opened this issue Jul 5, 2021 · 7 comments
Closed

Unit tests fail locally after pulling latest changes #656

WeilerP opened this issue Jul 5, 2021 · 7 comments
Assignees
Labels
bug Something isn't working

Comments

@WeilerP
Copy link
Member

WeilerP commented Jul 5, 2021

Description

After pulling the latest changes, the tests did not pass for me anymore when running

tox -e py38-linux

tox was using incompatible versions of umap-learn and numba (the same issue as in scverse/scanpy#1756 or theislab/scvelo#387) and a wrong version for wot. The issue with wot was

FAILED tests/test_external.py::TestWOTKernel::test_cost_matrices[good_shape] - TypeError: compute_transport_map() got an unexpected keyword argument 'cost_matrix'
FAILED tests/test_external.py::TestWOTKernel::test_last_time_point[connectivities] - TypeError: compute_transport_map() got an unexpected keyword argument 'cost_matrix'
FAILED tests/test_external.py::TestWOTKernel::test_no_connectivities - TypeError: compute_transport_map() got an unexpected keyword argument 'cost_matrix'
FAILED tests/test_external.py::TestWOTKernel::test_growth_rates[5] - TypeError: compute_transport_map() got an unexpected keyword argument 'cost_matrix'
FAILED tests/test_external.py::TestWOTKernel::test_normal_run - TypeError: compute_transport_map() got an unexpected keyword argument 'cost_matrix'
FAILED tests/test_external.py::TestWOTKernel::test_cost_matrices[X_pca] - TypeError: compute_transport_map() got an unexpected keyword argument 'cost_matrix'
FAILED tests/test_external.py::TestWOTKernel::test_last_time_point[diagonal] - TypeError: compute_transport_map() got an unexpected keyword argument 'cost_matrix'
FAILED tests/test_external.py::TestWOTKernel::test_growth_rates[3] - TypeError: compute_transport_map() got an unexpected keyword argument 'cost_matrix'
FAILED tests/test_external.py::TestWOTKernel::test_copy - TypeError: compute_transport_map() got an unexpected keyword argument 'cost_matrix'
FAILED tests/test_external.py::TestWOTKernel::test_cost_matrices[Ms] - TypeError: compute_transport_map() got an unexpected keyword argument 'cost_matrix'
FAILED tests/test_external.py::TestWOTKernel::test_last_time_point[uniform] - TypeError: compute_transport_map() got an unexpected keyword argument 'cost_matrix'
FAILED tests/test_external.py::TestWOTKernel::test_cost_matrices[None] - TypeError: compute_transport_map() got an unexpected keyword argument 'cost_matrix'

The keyword argument cost_matrix is not yet included in any tagged version of wot. I had to manually run

pip install -e '.[dev]'
pip install git+https://github.com/broadinstitute/wot

to make the tests pass again. On the latest CI run this morning, wot==1.0.8.post2 was installed and the above mentioned tests skipped. The CI run triggered by #653 installed the correct, i.e. latest, version of wot.

Versions:

cellrank==1.4.0+g1a71f63 scanpy==1.8.0 anndata==0.7.6 numpy==1.21.0 numba==0.52.0 scipy==1.7.0 pandas==1.2.5 pygpcca==1.0.2 scikit-learn==0.24.2 statsmodels==0.12.2 python-igraph==0.9.6 scvelo==0.2.3 pygam==0.8.0 matplotlib==3.4.2 seaborn==0.11.1

@WeilerP WeilerP added the bug Something isn't working label Jul 5, 2021
@michalk8
Copy link
Collaborator

michalk8 commented Jul 5, 2021

tox was using incompatible versions of umap-learn and numba (the same issue as in scverse/scanpy#1756 or theislab/scvelo#387) and a wrong version for wot. The issue with wot was

Strange, umap-learn should be only problem for 3.9 and the correct version of wot is on the dev branch (though not on master: https://github.com/theislab/cellrank/blob/dev/tox.ini#L100), will take a look. On master, wot is still being skipped.

@WeilerP
Copy link
Member Author

WeilerP commented Jul 5, 2021

Does tox -e py38-linux recheck the installed packages and down-/upgrade them if needed?

@michalk8
Copy link
Collaborator

michalk8 commented Jul 5, 2021

Does tox -e py38-linux recheck the installed packages and down-/upgrade them if needed?

Afaik not yet (see here, it's supposedly on the rewrite branch), you'd have to run tox -e <env> --recreate.

@WeilerP
Copy link
Member Author

WeilerP commented Jul 5, 2021

Strange, umap-learn should be only problem for 3.9 and the correct version of wot is on the dev branch (though not on master: https://github.com/theislab/cellrank/blob/dev/tox.ini#L100), will take a look. On master, wot is still being skipped.

Re the wot part: Doesn't this mean that after running pip install 'cellrank[external]' the WOTKernel cannot be used since wot<=1.0.8.post2 is installed ATM?

@michalk8
Copy link
Collaborator

michalk8 commented Jul 5, 2021

Re the wot part: Doesn't this mean that after running pip install 'cellrank[external]' the WOTKernel cannot be used since wot<=1.0.8.post2 is installed ATM?

Yes, that's why in docs we have the infor that it should be installed from git. In general, you can't specify git requirements in setup.py.

@WeilerP
Copy link
Member Author

WeilerP commented Jul 5, 2021

Yes, that's why in docs we have the infor that it should be installed from git. In general, you can't specify git requirements in setup.py.

Yes, a workaround would be to specify the requirements in a separate file, e.g. requirements_external.txt via

statot>=0.0.14
POT
wot @ git+https://github.com/broadinstitute/wot

and then parse this file in setup.py similar as it is done for "docs" ATM. Might be possible to define a single requirements.yml file and read the different packages from there.

@michalk8 michalk8 mentioned this issue Jul 5, 2021
3 tasks
@michalk8
Copy link
Collaborator

michalk8 commented Jul 5, 2021

closed via #655
let's continue the dev problems discussion in #628

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants