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

Change clifford to run with pytest #28

Merged
merged 3 commits into from
Feb 7, 2020
Merged

Conversation

eric-wieser
Copy link
Contributor

@eric-wieser eric-wieser commented Oct 14, 2019

We've dropped our nose dependency in clifford, and moved away from unittest-compatible tests in order to use pytest fixtures. Since python 2 is also not supported, the future module is no longer a dependency.

The upshot is our tests should be more granular.

We've dropped our nose dependency in clifford, and moved away from unittest-compatible tests in order to use pytest fixtures.

The upshot is our tests should be more granular.
@esc
Copy link
Member

esc commented Oct 14, 2019

There are still some import nose errors. Have you already released the version of clifford that has nose removed?

@eric-wieser
Copy link
Contributor Author

eric-wieser commented Oct 14, 2019

No, that hasn't been released yet. Do these tests run against both master and the released version, or only the released version?

If the former, I could revert my removal of nose and future from the required dependencies, and just let it install them needlessly for the master testing.

@esc
Copy link
Member

esc commented Oct 14, 2019

These tests usually only run against the latest released version.

@esc
Copy link
Member

esc commented Oct 14, 2019

For clifford specifically, the program will obtain all git tags from the 'blessed' remote and select the 'latest' one. The code is here:

https://github.com/numba/numba-integration-testing/blob/master/switchboard.py#L115

@eric-wieser
Copy link
Contributor Author

It sounds then like the correct action is to leave this open until clifford pushes its next tag?

@esc
Copy link
Member

esc commented Oct 14, 2019

@eric-wieser probably, yes.

@eric-wieser eric-wieser reopened this Nov 19, 2019
@eric-wieser
Copy link
Contributor Author

Clifford did a 1.1.0 release, although now I expect this to fail due to a missing conda-forge:sparse dependency, which I'm not sure how to add.

@esc
Copy link
Member

esc commented Feb 6, 2020

@eric-wieser thanks for submitting this, I'll try to re-run this PR now.

@esc esc closed this Feb 6, 2020
@esc esc reopened this Feb 6, 2020
@esc
Copy link
Member

esc commented Feb 6, 2020

@eric-wieser for reference, you can add a string like "-c conda-forge sparse", to install sparse from conda-forge.

@esc
Copy link
Member

esc commented Feb 6, 2020

I think the clifford tests are going through fine, but that is because sparse is listed as dependency in the setup.py and so we get:

Installed /home/circleci/repo/miniconda3/envs/clifford/lib/python3.8/site-packages/clifford-1.2.0-py3.8.egg
Processing dependencies for clifford==1.2.0
Searching for sparse
Reading https://pypi.org/simple/sparse/
Downloading https://files.pythonhosted.org/packages/08/73/34946ead922c072d7981be5b917f5871961a1b3bab4bc43dcd861de9ac23/sparse-0.9.1-py2.py3-none-any.whl#sha256=5d7e9bab68b20f63ba6e8c593e46864ebef3a0842fa26fb32b71e743e6c5c99e
Best match: sparse 0.9.1
Processing sparse-0.9.1-py2.py3-none-any.whl
Installing sparse-0.9.1-py2.py3-none-any.whl to /home/circleci/repo/miniconda3/envs/clifford/lib/python3.8/site-packages
Adding sparse 0.9.1 to easy-install.pth file

switchboard.py Show resolved Hide resolved
Co-Authored-By: Valentin Haenel <esc@users.noreply.github.com>
@esc
Copy link
Member

esc commented Feb 6, 2020

@eric-wieser thanks for submitting a fix for this, will merge as soon as CI goes green!

@esc
Copy link
Member

esc commented Feb 6, 2020

It looks like it has passed, but actually I see:

= 1 failed, 315 passed, 29 skipped, 8 xfailed, 1 xpassed, 5 warnings in 394.96s (0:06:34) =

@esc
Copy link
Member

esc commented Feb 6, 2020

Looks like we may have to wait for: conda/conda#9665

@esc
Copy link
Member

esc commented Feb 7, 2020

I have merged a workaround for the conda run bug: numba/texasbbq#7 @eric-wieser would it be possible to rebase this PR onto master (or merge master into it). This will hopefully show us the true state of these integration-tests.

@eric-wieser eric-wieser closed this Feb 7, 2020
@eric-wieser eric-wieser reopened this Feb 7, 2020
@eric-wieser
Copy link
Contributor Author

^ That should have the effect of re-running against master

@esc
Copy link
Member

esc commented Feb 7, 2020

@eric-wieser yeah, I thought so too, but apparently CircleCI doesn't seem to operate like that.

@esc
Copy link
Member

esc commented Feb 7, 2020

I triggered it manually now.

@eric-wieser
Copy link
Contributor Author

eric-wieser commented Feb 7, 2020

Test failure is real, AttributeError: module 'numba' has no attribute 'numpy_support'.

Looks like the numba API changed on us.

@esc
Copy link
Member

esc commented Feb 7, 2020

@eric-wieser indeed, we are in the middle of a 'refactor the world' update in Numba: numba/numba#5197

It is very likely that this broke clifford and lead to the failures you are seeing now. I would suggest to collect all the places where the Numba refactor broke clifford and submit these in an issue for discussion on the Numba issue tracker. This will allow us to decide if we broke a 'public' API and we need to fix Numba --- OR --- if clifford was using a private API in the first place and the fix needs to happen there.

And thanks very much for your continued understanding, patience and persistence --- we appreciate it!

@eric-wieser
Copy link
Contributor Author

At any rate, this PR should be good to merge, right?

@esc
Copy link
Member

esc commented Feb 7, 2020

@eric-wieser indeed! The errors are being reported again and the clifford tests no longer fail due to a missing pytest dependency!

@esc esc merged commit 961ffb2 into numba:master Feb 7, 2020
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.

None yet

2 participants