Skip to content

Commit

Permalink
Update CI (keras-team#159)
Browse files Browse the repository at this point in the history
### Summary
Update CI, basically separate every test component.
- PEP8 (no dependencies installed at all)
- PyTests (install test dependencies), runs for Python 2.6, 3.6 and Keras PyPI and Keras HEAD
- Imports (install package dependencies), runs for Python 2.6, 3.6. Basically imports the package and modules as a check that nothing breaks.

I also change where some of the logic happens. Before there was some logic in `travis.yml` that in my opinion `shuoldn't` be there but in `setup.py`, in this way the user can actually execute the same that travis is doing by `pip install -e .[tests] && pytest tests` or  `pip install -e .[pep8] && flake8`. So much easier to debug without the need to push and create a pull request to reproduce travis execution (up to some complexity).

This should lead to a more robust check and specially avoid the error given during the release of 1.0.6. That error for example will be catch in two places now: 
- PEP8: as an unused import
- Imports: since pandas is not installed on that check.

NOTE: The phase caching the bug in the 1.0.6 should not be the Pytest phase, that actually should work. Since pandas is needed for unit testing.

By running these tests on the current state of the repo I already found several bugs in the code, several overhead variables being assigned but unused (specially in tests) and like 30 plus PEP8 violations.

### Related Issues
keras-team#154 

### PR Overview

- [ n] This PR requires new unit tests [y/n] (make sure tests are included)
- [ n] This PR requires to update the documentation [y/n] (make sure the docs are up-to-date)
- [ y] This PR is backwards compatible [y/n]
- [ n] This PR changes the current API [y/n] (all API changes need to be approved by fchollet)
  • Loading branch information
rragundez authored and Frédéric Branchaud-Charron committed Feb 3, 2019
1 parent dd35790 commit 888abbc
Show file tree
Hide file tree
Showing 16 changed files with 75 additions and 94 deletions.
59 changes: 35 additions & 24 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3,20 +3,37 @@ dist: trusty
language: python
matrix:
include:
# check code style and Python 2.7
- python: 2.7
env: TEST_MODE=PEP8
# run tests with keras from source and Python 2.7
- python: 2.7
env: KERAS_HEAD=true
env: TEST_MODE=TESTS
# run tests with keras from source and Python 3.6
- python: 3.6
env: KERAS_HEAD=true
env: TEST_MODE=TESTS
# run tests with keras from PyPI and Python 2.7
- python: 2.7
env: TEST_MODE=TESTS
# run tests with keras from PyPI and Python 3.6
- python: 3.6
install:
# code below is taken from http://conda.pydata.org/docs/travis.html
env: TEST_MODE=TESTS
# run import test and Python 2.7
- python: 2.7
env: TEST_MODE=IMPORTS
# run import test and Python 3.6
- python: 3.6
env: TEST_MODE=IMPORTS


before_install:
- sudo apt-get update
# We do this conditionally because it saves us some downloading if the
# version is the same.
- if [[ "$TRAVIS_PYTHON_VERSION" == "2.7" ]]; then
wget https://repo.continuum.io/miniconda/Miniconda-latest-Linux-x86_64.sh -O miniconda.sh;
wget https://repo.continuum.io/miniconda/Miniconda2-latest-Linux-x86_64.sh -O miniconda.sh;
else
wget https://repo.continuum.io/miniconda/Miniconda3-latest-Linux-x86_64.sh -O miniconda.sh;
fi
Expand All @@ -27,32 +44,26 @@ install:
- conda update -q conda
# Useful for debugging any issues with conda
- conda info -a

- conda create -q -n test-environment python=$TRAVIS_PYTHON_VERSION pytest pandas
- conda create -q -n test-environment python=$TRAVIS_PYTHON_VERSION
- source activate test-environment
- pip install --only-binary=numpy,scipy numpy nose scipy keras

# set library path
- export LD_LIBRARY_PATH=$HOME/miniconda/envs/test-environment/lib/:$LD_LIBRARY_PATH

# install PIL for image tests
- if [[ "$TRAVIS_PYTHON_VERSION" == "2.7" ]]; then
conda install pil;
elif [[ "$TRAVIS_PYTHON_VERSION" == "3.6" ]]; then
conda install Pillow;
fi
install:
- if [[ $KERAS_HEAD == "true" ]]; then
pip install --no-deps git+https://github.com/keras-team/keras.git ;
pip install --no-deps git+https://github.com/keras-team/keras.git --upgrade;
fi
- if [[ "$TEST_MODE" == "PEP8" ]]; then
pip install -e .[pep8];
elif [[ "$TEST_MODE" == "TESTS" ]]; then
pip install -e .[tests];
elif [[ "$TEST_MODE" == "IMPORTS" ]]; then
pip install .;
fi
- pip install -e .[tests]

# install TensorFlow (CPU version).
- pip install tensorflow==1.7

# command to run tests
script:
- if [[ "$TEST_MODE" == "PEP8" ]]; then
PYTHONPATH=$PWD:$PYTHONPATH py.test --pep8 -m pep8 -n0;
else
PYTHONPATH=$PWD:$PYTHONPATH py.test tests/ --cov-config .coveragerc --cov=keras_preprocessing tests/;
flake8 -v --count;
elif [[ "$TEST_MODE" == "TESTS" ]]; then
py.test tests --cov-config .coveragerc --cov=keras_preprocessing tests;
elif [[ "$TEST_MODE" == "IMPORTS" ]]; then
python -c "import keras_preprocessing; from keras_preprocessing import image; from keras_preprocessing import sequence; from keras_preprocessing import text";
fi
1 change: 1 addition & 0 deletions keras_preprocessing/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,4 +39,5 @@ def get_keras_submodule(name):
elif name == 'utils':
return _KERAS_UTILS


__version__ = '1.0.8'
4 changes: 1 addition & 3 deletions keras_preprocessing/image/__init__.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
"""Enables dynamic setting of underlying Keras module.
"""
from __future__ import absolute_import
from __future__ import division
from __future__ import print_function

# flake8: noqa:F401
from .affine_transformations import *
from .dataframe_iterator import DataFrameIterator
from .directory_iterator import DirectoryIterator
Expand Down
8 changes: 1 addition & 7 deletions keras_preprocessing/image/affine_transformations.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,19 +4,13 @@
from __future__ import division
from __future__ import print_function

import os
import re
import warnings

import numpy as np

from .utils import (array_to_img,
img_to_array)

try:
import scipy
# scipy.linalg cannot be accessed until explicitly imported
from scipy import linalg
# scipy.ndimage cannot be accessed until explicitly imported
from scipy import ndimage
except ImportError:
Expand Down Expand Up @@ -327,7 +321,7 @@ def apply_affine_transform(x, theta=0, tx=0, ty=0, shear=0, zx=1, zy=1,
final_affine_matrix = transform_matrix[:2, :2]
final_offset = transform_matrix[:2, 2]

channel_images = [scipy.ndimage.interpolation.affine_transform(
channel_images = [ndimage.interpolation.affine_transform(
x_channel,
final_affine_matrix,
final_offset,
Expand Down
9 changes: 2 additions & 7 deletions keras_preprocessing/image/dataframe_iterator.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,8 @@
import os
import warnings

import numpy as np

from .iterator import BatchFromFilesMixin, Iterator
from .utils import (array_to_img,
get_extension,
img_to_array,
load_img)
from .utils import get_extension


class DataFrameIterator(BatchFromFilesMixin, Iterator):
Expand Down Expand Up @@ -220,7 +215,7 @@ def remove_classes(labels, classes):
else:
raise TypeError(
"Expect string, list or tuple but found {} in {} column "
.format(type(x), y_col)
.format(type(labels), y_col)
)

if classes:
Expand Down
5 changes: 1 addition & 4 deletions keras_preprocessing/image/directory_iterator.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,7 @@
import numpy as np

from .iterator import BatchFromFilesMixin, Iterator
from .utils import (array_to_img,
img_to_array,
_list_valid_filenames_in_directory,
load_img)
from .utils import _list_valid_filenames_in_directory


class DirectoryIterator(BatchFromFilesMixin, Iterator):
Expand Down
3 changes: 1 addition & 2 deletions keras_preprocessing/image/image_data_generator.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
# scipy.linalg cannot be accessed until explicitly imported
from scipy import linalg
# scipy.ndimage cannot be accessed until explicitly imported
from scipy import ndimage
except ImportError:
scipy = None

Expand Down Expand Up @@ -957,6 +956,6 @@ def fit(self, x,
flat_x = np.reshape(
x, (x.shape[0], x.shape[1] * x.shape[2] * x.shape[3]))
sigma = np.dot(flat_x.T, flat_x) / flat_x.shape[0]
u, s, _ = scipy.linalg.svd(sigma)
u, s, _ = linalg.svd(sigma)
s_inv = 1. / np.sqrt(s[np.newaxis] + self.zca_epsilon)
self.principal_components = (u * s_inv).dot(u.T)
1 change: 1 addition & 0 deletions keras_preprocessing/image/iterator.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
from __future__ import division
from __future__ import print_function

import os
import threading
import numpy as np
from keras_preprocessing import get_keras_submodule
Expand Down
2 changes: 1 addition & 1 deletion keras_preprocessing/image/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ def _recursive_list(subpath):
warnings.warn('Using ".tiff" files with multiple bands '
'will cause distortion. Please verify your output.')
if get_extension(fname) in white_list_formats:
yield root, fname
yield root, fname


def _list_valid_filenames_in_directory(directory, white_list_formats, split,
Expand Down
4 changes: 2 additions & 2 deletions keras_preprocessing/sequence.py
Original file line number Diff line number Diff line change
Expand Up @@ -384,15 +384,15 @@ def get_config(self):
data = self.data.tolist()
try:
json_data = json.dumps(data)
except:
except TypeError:
raise TypeError('Data not JSON Serializable:', data)

targets = self.targets
if type(self.targets).__module__ == np.__name__:
targets = self.targets.tolist()
try:
json_targets = json.dumps(targets)
except:
except TypeError:
raise TypeError('Targets not JSON Serializable:', targets)

return {
Expand Down
3 changes: 2 additions & 1 deletion keras_preprocessing/text.py
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,8 @@ def hashing_trick(text, n,
if hash_function is None:
hash_function = hash
elif hash_function == 'md5':
hash_function = lambda w: int(md5(w.encode()).hexdigest(), 16)
def hash_function(w):
return int(md5(w.encode()).hexdigest(), 16)

seq = text_to_word_sequence(text,
filters=filters,
Expand Down
18 changes: 8 additions & 10 deletions pytest.ini → setup.cfg
Original file line number Diff line number Diff line change
@@ -1,18 +1,16 @@
# Configuration of py.test
[pytest]
addopts=-v
-n 2
--durations=20

[tool:pytest]]
addopts=-v -n 2 --durations=20
# Do not run tests in the build folder
norecursedirs= build
norecursedirs=build

[flake8]
# Use 85 as max line length in PEP8 test.
pep8maxlinelength=85

max-line-length=85
# do not run pep8 test in the build folder
exclude=build
# PEP-8 The following are ignored:
# E731 do not assign a lambda expression, use a def
# E402 module level import not at top of file

pep8ignore=* E731 \
* E402 \
* E402 \
10 changes: 8 additions & 2 deletions setup.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import sys

from setuptools import setup
from setuptools import find_packages

Expand Down Expand Up @@ -33,10 +35,14 @@
install_requires=['numpy>=1.9.1',
'six>=1.9.0'],
extras_require={
'tests': ['pytest',
'pytest-pep8',
'tests': ['pandas',
'Pillow' if sys.version_info >= (3, 0) else 'pillow',
'tensorflow==1.7', # CPU version
'keras',
'pytest',
'pytest-xdist',
'pytest-cov'],
'pep8': ['flake8'],
'image': ['scipy>=0.14',
'Pillow>=5.2.0'],
},
Expand Down
25 changes: 10 additions & 15 deletions tests/image_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,17 +4,9 @@
import os
import tempfile
import shutil
import keras
import pandas as pd
import random

# TODO: remove the 3 lines below once the Keras release
# is configured to use keras_preprocessing
import keras_preprocessing
keras_preprocessing.set_keras_submodules(
backend=keras.backend, utils=keras.utils)

# This enables this import
from keras_preprocessing import image


Expand Down Expand Up @@ -262,7 +254,7 @@ def test_image_data_generator_with_validation_split(self):

def test_image_data_generator_with_split_value_error(self):
with pytest.raises(ValueError):
generator = image.ImageDataGenerator(validation_split=5)
image.ImageDataGenerator(validation_split=5)

def test_image_data_generator_invalid_data(self):
generator = image.ImageDataGenerator(
Expand Down Expand Up @@ -535,12 +527,14 @@ def test_dataframe_iterator(self, tmpdir):
dtype=str)
batch_x, batch_y = next(df_multiple_y_iterator)
with pytest.raises(TypeError):
df_multiple_y_iterator = generator.flow_from_dataframe(
generator.flow_from_dataframe(
df_regression, str(tmpdir), y_col=["col1", "col2"],
class_mode="other")
class_mode="other"
)
with pytest.raises(TypeError):
df_single_y_iterator = generator.flow_from_dataframe(
df_regression, str(tmpdir), y_col="col1", class_mode="other")
generator.flow_from_dataframe(
df_regression, str(tmpdir), y_col="col1", class_mode="other"
)
# check number of classes and images
assert len(df_iterator.class_indices) == num_classes
assert len(df_iterator.classes) == count
Expand Down Expand Up @@ -913,12 +907,12 @@ def test_dataframe_iterator_with_drop_duplicates(self, tmpdir):
# create iterators
generator = image.ImageDataGenerator()
df_drop_iterator = generator.flow_from_dataframe(
df2, str(tmpdir), class_mode=None, drop_duplicates=True)
df, str(tmpdir), class_mode=None, drop_duplicates=True)
df_no_drop_iterator = generator.flow_from_dataframe(
df2, str(tmpdir), class_mode=None, drop_duplicates=False)

# Test drop_duplicates
assert df_drop_iterator.n == len(set(input_filenames2))
assert df_drop_iterator.n == len(set(input_filenames))
assert df_no_drop_iterator.n == len(input_filenames2)

def test_dataframe_iterator_with_subdirs(self, tmpdir):
Expand Down Expand Up @@ -1230,5 +1224,6 @@ def test_load_img(self, tmpdir):
loaded_im = image.load_img(filename_rgb, target_size=(25, 25),
interpolation="unsupported")


if __name__ == '__main__':
pytest.main([__file__])
8 changes: 0 additions & 8 deletions tests/sequence_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,6 @@
from numpy.testing import assert_equal
from numpy.testing import assert_raises

import keras

# TODO: remove the 3 lines below once the Keras release
# is configured to use keras_preprocessing
import keras_preprocessing
keras_preprocessing.set_keras_submodules(
backend=keras.backend, utils=keras.utils)

from keras_preprocessing import sequence


Expand Down
9 changes: 1 addition & 8 deletions tests/text_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,6 @@
import pytest

import keras

# TODO: remove the 3 lines below once the Keras release
# is configured to use keras_preprocessing
import keras_preprocessing
keras_preprocessing.set_keras_submodules(
backend=keras.backend, utils=keras.utils)

from keras_preprocessing import text
from collections import OrderedDict

Expand Down Expand Up @@ -54,7 +47,7 @@ def test_tokenizer():
tokenizer.fit_on_sequences(sequences)

for mode in ['binary', 'count', 'tfidf', 'freq']:
matrix = tokenizer.texts_to_matrix(sample_texts, mode)
tokenizer.texts_to_matrix(sample_texts, mode)


def test_tokenizer_serde_no_fitting():
Expand Down

0 comments on commit 888abbc

Please sign in to comment.