Skip to content

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

Merged
NicolasHug merged 1 commit intoscikit-learn:masterfrom
oleksandr-pavlyk:import-cython-in-tests
May 29, 2019
Merged

FIX Lazy cython import for pytest to work without cython#13980
NicolasHug merged 1 commit intoscikit-learn:masterfrom
oleksandr-pavlyk:import-cython-in-tests

Conversation

@oleksandr-pavlyk
Copy link
Copy Markdown
Contributor

@oleksandr-pavlyk oleksandr-pavlyk commented May 29, 2019

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
Copy Markdown
Contributor Author

oleksandr-pavlyk commented May 29, 2019

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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Member

NicolasHug commented May 29, 2019

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

2 more reviewers

@rth rth rth approved these changes

@NicolasHug NicolasHug NicolasHug approved these changes

Reviewers whose approvals may not affect merge requirements

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