[MRG + 1] Fix skip test lz4 not installed#695
Conversation
For old version of lz4, installed with conda, lz4.frame module does not have a LZ4FrameFile object. Ensure that this object is present to allow lz4 support.
Codecov Report
@@ Coverage Diff @@
## master #695 +/- ##
==========================================
- Coverage 95.12% 95.03% -0.09%
==========================================
Files 40 40
Lines 5663 5665 +2
==========================================
- Hits 5387 5384 -3
- Misses 276 281 +5
Continue to review full report at Codecov.
|
aabadie
left a comment
There was a problem hiding this comment.
This is in a corner case of Travis CI build matrix: LZ4 is always installed when Numpy is installed and the test_joblib_compression_formats is skipped when numpy is not installed but not when LZ4 is not installed and numpy is.
I'm fine with your changes but the line under the added elif in test_numpy_pickle is not covered by the test.
It can be fixed by changing the Travis CI build matrix:
Maybe skip installation of LZ4 when Numpy is installed and Python version is 3.5.
| elif cmethod == 'lz4' and with_lz4.args[0]: | ||
| # Skip the test if lz4 is not installed. We here use the with_lz4 | ||
| # skipif fixture whose argument is True when lz4 is not installed | ||
| raise SkipTest("lz4 is not installed.") |
There was a problem hiding this comment.
This line is not covered by the CI now.
There was a problem hiding this comment.
Ok it should be covered with my last
|
The coverage diff highlights lines that cannot be made covered in a deterministic manner and are unrelated to LZ4. |
|
And the diff coverage is low because the build without |
| create_new_conda_env | ||
|
|
||
| # Install py.test timeout to fasten failure in deadlocking tests | ||
| PIP_INSTALL_PACKAGES="pytest-timeout" |
There was a problem hiding this comment.
I just refactored all the pip install in one so I had to put this one in the list.
I think it makes think easier to read but I can put it directly in the pip install if you prefer.
Maybe skipping lz4 with python 3.5 was not a good idea. I'm fine with an extra line in the matrix. |
aabadie
left a comment
There was a problem hiding this comment.
LGTM + 1. Let's merge this one.
* tag '0.12': (116 commits) Release 0.12 typo typo typo ENH add initializer limiting n_threads for C-libs (joblib#701) DOC better parallel docstring (joblib#704) [MRG] Nested parallel call thread bomb mitigation (joblib#700) MTN vendor loky2.1.3 (joblib#699) Make it possible to configure the reusable executor workers timeout (joblib#698) MAINT increase timeouts to make test more robust on travis DOC: use the .joblib extension instead of .pkl (joblib#697) [MRG] Fix exception handling in nested parallel calls (joblib#696) Fix skip test lz4 not installed (joblib#695) [MRG] numpy_pickle: several enhancements (joblib#626) Introduce Parallel.__call__ backend callbacks (joblib#689) Add distributed on readthedocs (joblib#686) Support registration of external backends (joblib#655) [MRG] Add a dask.distributed example (joblib#613) ENH use cloudpickle to pickle interactively defined callable (joblib#677) CI freeze the version of sklearn0.19.1 and scipy1.0.1 (joblib#685) ...
* releases: (121 commits) Release 0.12.1 fix kwonlydefaults key error in filter_args (joblib#715) MNT fix some "undefined name" flake8 warnings (joblib#713). from importlib import reload for Python 3 (joblib#675) MTN vendor loky2.1.4 (joblib#708) Release 0.12 typo typo typo ENH add initializer limiting n_threads for C-libs (joblib#701) DOC better parallel docstring (joblib#704) [MRG] Nested parallel call thread bomb mitigation (joblib#700) MTN vendor loky2.1.3 (joblib#699) Make it possible to configure the reusable executor workers timeout (joblib#698) MAINT increase timeouts to make test more robust on travis DOC: use the .joblib extension instead of .pkl (joblib#697) [MRG] Fix exception handling in nested parallel calls (joblib#696) Fix skip test lz4 not installed (joblib#695) [MRG] numpy_pickle: several enhancements (joblib#626) Introduce Parallel.__call__ backend callbacks (joblib#689) ...
No description provided.