8000 FIX Lazy cython import for pytest to work without cython by oleksandr-pavlyk · Pull Request #13980 · scikit-learn/scikit-learn · GitHub
[go: up one dir, main page]
Skip to content

FIX Lazy cython import for pytest to work without cython #13980

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
May 29, 2019
Merged

FIX Lazy cython import for pytest to work without cython #13980

merged 1 commit into from
May 29, 2019

Conversation

oleksandr-pavlyk
Copy link
Contributor

Present of import atop the test file caused pytest to fail collecting
tests if the testing environment did not have cython installed.

@ogrisel @jeremiedbb

@oleksandr-pavlyk
Copy link
Contributor Author

To reproduce, I created an empty code environment with python 3.7, pip installed into it scikit-learn and pytest:

conda create -q -y -n issue_13980 python=3.7 pytest
conda activate issue_13980
pip --no-cache-dir install scikit-learn==0.21.2

I then run pytest and an error during collecting step of pytest run:

ModuleNotFoundError: No module named 'Cython' 

Installing collected packages: joblib, numpy, scipy, scikit-learn
Successfully installed joblib-0.13.2 numpy-1.16.4 scikit-learn-0.21.2 scipy-1.3.0
[12:04:31 vmlin test_tmp]$ pytest --pyargs sklearn
========================================= test session starts ==========================================
platform linux -- Python 3.7.3, pytest-4.5.0, py-1.8.0, pluggy-0.11.0
rootdir: /localdisk/work/opavlyk/saved_homedir/git/test_tmp
collected 6476 items / 1 errors / 2 skipped / 6473 selected

================================================ ERRORS ================================================
________________________________ ERROR collecting tests/test_common.py _________________________________
ImportError while importing test module '/localdisk/work/opavlyk/miniconda3_cb3/envs/issue_13980/lib/python3.7/site-packages/sklearn/tests/test_common.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
../../../miniconda3_cb3/envs/issue_13980/lib/python3.7/site-packages/sklearn/tests/test_common.py:52: in <module>
    all_estimators()
../../../miniconda3_cb3/envs/issue_13980/lib/python3.7/site-packages/sklearn/utils/testing.py:660: in all_estimators
    module = __import__(modname, fromlist="dummy")
../../../miniconda3_cb3/envs/issue_13980/lib/python3.7/site-packages/sklearn/linear_model/setup.py:5: in <module>
    from Cython import Tempita
E   ModuleNotFoundError: No module named 'Cython'

NicolasHug
NicolasHug approved these changes May 29, 2019
View reviewed changes
Copy link
Member
@NicolasHug NicolasHug left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM otherwise.

Can you confirm there is not other occurrences?

rth
rth approved these changes May 29, 2019
View reviewed changes
Copy link
Member
@rth rth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you confirm there is not other occurrences?

Can confirm there are none.

Good to merge once the above PEP8 change is accepted.

@oleksandr-pavlyk
BUG: moved from Cython import Tempita from top file to where needed
60df40a 
Present of import atop the test file caused pytest to fail collecting
tests if the testing environment did not have cython installed.
@oleksandr-pavlyk oleksandr-pavlyk force-pushed the import-cython-in-tests branch from 37befa7 to 60df40a Compare May 29, 2019 18:18
@thomasjpfan thomasjpfan mentioned this pull request May 29, 2019
Closed
@NicolasHug NicolasHug changed the title BUG: moved from Cython import Tempita from top file to where needed FIX Lazy cython import for pytest to work without cython May 29, 2019
@NicolasHug NicolasHug merged commit 7ee46f1 into scikit-learn:master May 29, 2019
@NicolasHug
Copy link
Member

Thanks @oleksandr-pavlyk

@oleksandr-pavlyk oleksandr-pavlyk deleted the import-cython-in-tests branch May 29, 2019 19:08
@jnothman jnothman added this to the 0.21.3 milestone May 30, 2019
@jeremiedbb jeremiedbb mentioned this pull request Oct 21, 2019
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rth rth rth approved these changes

@NicolasHug NicolasHug NicolasHug approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
0.21.3
Development

Successfully merging this pull request may close these issues.
4 participants
@oleksandr-pavlyk @NicolasHug @rth @jnothman
0