From a563a8446e8b7c568cefa18225d2156f75ae7c5b Mon Sep 17 00:00:00 2001 From: olegsinyavskiy Date: Thu, 14 Jan 2016 08:32:37 -0800 Subject: [PATCH] Add an automatic PEP8 check on the pull request submission:\n - ignore most of the errors to avoid disrupting others\n - add a separate job to avoid confusion that all jobs fail because of a single pep error\n - fix few small pep errors --- .travis.yml | 9 ++++- CONTRIBUTING.md | 15 +++++--- docs/autogen.py | 2 +- keras/models.py | 2 +- keras/preprocessing/image.py | 37 ++++++++----------- keras/preprocessing/sequence.py | 2 +- keras/preprocessing/text.py | 3 +- pytest.ini | 31 ++++++++++++++++ .../test_temporal_data_tasks.py | 4 +- tests/keras/layers/test_convolutional.py | 1 - tests/keras/layers/test_core.py | 8 ++-- tests/keras/layers/test_recurrent.py | 1 - tests/keras/preprocessing/test_image.py | 2 +- tests/keras/test_models.py | 1 + 14 files changed, 77 insertions(+), 41 deletions(-) diff --git a/.travis.yml b/.travis.yml index 51d6d7c9d45e..374ad72d54b9 100644 --- a/.travis.yml +++ b/.travis.yml @@ -12,7 +12,9 @@ matrix: - python: 2.7 env: KERAS_BACKEND=tensorflow - python: 2.7 - env: KERAS_BACKEND=theano INTEGRATION_TESTS=true + env: KERAS_BACKEND=theano TEST_MODE=INTEGRATION_TESTS + - python: 2.7 + env: KERAS_BACKEND=theano TEST_MODE=PEP8 install: # code below is taken from http://conda.pydata.org/docs/travis.html # We do this conditionally because it saves us some downloading if the @@ -33,6 +35,7 @@ install: - conda create -q -n test-environment python=$TRAVIS_PYTHON_VERSION numpy scipy matplotlib pandas pytest h5py - source activate test-environment - pip install pytest-cov python-coveralls pytest-xdist coverage==3.7.1 #we need this version of coverage for coveralls.io to work + - pip install pep8 pytest-pep8 - pip install git+git://github.com/Theano/Theano.git # install PIL for preprocessing tests @@ -57,8 +60,10 @@ script: # set up keras backend - sed -i -e 's/"backend":[[:space:]]*"[^"]*/"backend":\ "'$KERAS_BACKEND'/g' ~/.keras/keras.json; - echo -e "Running tests with the following config:\n$(cat ~/.keras/keras.json)" - - if [[ "$INTEGRATION_TESTS" == "true" ]]; then + - if [[ "$TEST_MODE" == "INTEGRATION_TESTS" ]]; then PYTHONPATH=$PWD:$PYTHONPATH py.test tests/integration_tests; + elif [[ "$TEST_MODE" == "PEP8" ]]; then + PYTHONPATH=$PWD:$PYTHONPATH py.test --pep8 -m pep8 -n0; else PYTHONPATH=$PWD:$PYTHONPATH py.test tests/ --ignore=tests/integration_tests; fi diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index b2c7f9f82737..e8df56657418 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -36,24 +36,29 @@ We love pull requests. Here's a quick guide: 1. If your PR introduces a change in functionality, make sure you start by opening an issue to discuss whether the change should be made, and how to handle it. This will save you from having your PR closed down the road! Of course, if your PR is a simple bug fix, you don't need to do that. -2. Write the code. This is the hard part! We use PEP8 syntax conventions, but we aren't dogmatic when it comes to line length. Make sure your lines stay reasonably sized, though. To make your life easier, we recommend installing a PEP8 linter. +2. Write the code. This is the hard part! 3. Make sure any new function or class you introduce has proper docstrings. Make sure any code you touch still has up-to-date docstrings and documentation. 4. Write tests. Your code should have full unit test coverage. If you want to see your PR merged promptly, this is crucial. 5. Run our test suite locally. It's easy: from the Keras folder, simply run: `py.test tests/`. - - You will need to install `pytest`, `coveralls`, `pytest-cov`, `pytest-xdist`: `pip install pytest pytest-cov python-coveralls pytest-xdist` + - You will need to install `pytest`, `coveralls`, `pytest-cov`, `pytest-xdist`: `pip install pytest pytest-cov python-coveralls pytest-xdist pep8 pytest-pep8` 6. Make sure all tests are passing: - with the Theano backend, on Python 2.7 and Python 3.5 - with the TensorFlow backend, on Python 2.7 -7. When committing, use appropriate, descriptive commit messages. Make sure that your branch history is not a string of "bug fix", "fix", "oops", etc. When submitting your PR, squash your commits into a single commit with an appropriate commit message, to make sure the project history stays clean and readable. See ['rebase and squash'](http://rebaseandsqua.sh/) for technical help on how to squash your commits. +7. We use PEP8 syntax conventions, but we aren't dogmatic when it comes to line length. Make sure your lines stay reasonably sized, though. To make your life easier, we recommend running a PEP8 linter: + - Install PEP8 packages: `pip install pep8 pytest-pep8 autopep8` + - Run a standalone PEP8 check: `py.test --pep8 -m pep8` + - You can automatically fix some PEP8 error by running: `autopep8 -i --select ` for example: `autopep8 -i --select E128 tests/keras/backend/test_backends.py` -8. Update the documentation. If introducing new functionality, make sure you include code snippets demonstrating the usage of your new feature. +8. When committing, use appropriate, descriptive commit messages. Make sure that your branch history is not a string of "bug fix", "fix", "oops", etc. When submitting your PR, squash your commits into a single commit with an appropriate commit message, to make sure the project history stays clean and readable. See ['rebase and squash'](http://rebaseandsqua.sh/) for technical help on how to squash your commits. -9. Submit your PR. If your changes have been approved in a previous discussion, and if you have complete (and passing) unit tests, your PR is likely to be merged promptly. Otherwise, well... +9. Update the documentation. If introducing new functionality, make sure you include code snippets demonstrating the usage of your new feature. + +10. Submit your PR. If your changes have been approved in a previous discussion, and if you have complete (and passing) unit tests, your PR is likely to be merged promptly. Otherwise, well... ## Adding new examples diff --git a/docs/autogen.py b/docs/autogen.py index 129c5acaf028..c1b6011005a5 100644 --- a/docs/autogen.py +++ b/docs/autogen.py @@ -80,7 +80,7 @@ def get_method_signature(method): for a in args: st += str(a) + ', ' for a, v in kwargs: - if type(v) == str: + if type(v) == str: v = '\'' + v + '\'' elif type(v) == unicode: v = 'u\'' + v + '\'' diff --git a/keras/models.py b/keras/models.py index 13d7a108f20d..a2706df14e0b 100644 --- a/keras/models.py +++ b/keras/models.py @@ -385,7 +385,7 @@ def get_json_type(obj): # if obj is any numpy type if type(obj).__module__ == np.__name__: - return obj.item(); + return obj.item() # if obj is a python 'type' if type(obj).__name__ == type.__name__: diff --git a/keras/preprocessing/image.py b/keras/preprocessing/image.py index 793baa1a3c3f..f7eeda076902 100644 --- a/keras/preprocessing/image.py +++ b/keras/preprocessing/image.py @@ -7,7 +7,8 @@ from os import listdir from os.path import isfile, join -import random, math +import random +import math from six.moves import range ''' @@ -74,8 +75,6 @@ def random_zoom(x, rg, fill_mode="nearest", cval=0.): return x # shape of result will be different from shape of input! - - def array_to_img(x, scale=True): from PIL import Image x = x.transpose(1, 2, 0) @@ -113,9 +112,8 @@ def load_img(path, grayscale=False): def list_pictures(directory, ext='jpg|jpeg|bmp|png'): - return [join(directory,f) for f in listdir(directory) \ - if isfile(join(directory,f)) and re.match('([\w]+\.(?:' + ext + '))', f)] - + return [join(directory,f) for f in listdir(directory) + if isfile(join(directory,f)) and re.match('([\w]+\.(?:' + ext + '))', f)] class ImageDataGenerator(object): @@ -124,24 +122,23 @@ class ImageDataGenerator(object): realtime data augmentation. ''' def __init__(self, - featurewise_center=True, # set input mean to 0 over the dataset - samplewise_center=False, # set each sample mean to 0 - featurewise_std_normalization=True, # divide inputs by std of the dataset - samplewise_std_normalization=False, # divide each input by its std - - zca_whitening=False, # apply ZCA whitening - rotation_range=0., # degrees (0 to 180) - width_shift_range=0., # fraction of total width - height_shift_range=0., # fraction of total height - horizontal_flip=False, - vertical_flip=False, - ): + featurewise_center=True, # set input mean to 0 over the dataset + samplewise_center=False, # set each sample mean to 0 + featurewise_std_normalization=True, # divide inputs by std of the dataset + samplewise_std_normalization=False, # divide each input by its std + + zca_whitening=False, # apply ZCA whitening + rotation_range=0., # degrees (0 to 180) + width_shift_range=0., # fraction of total width + height_shift_range=0., # fraction of total height + horizontal_flip=False, + vertical_flip=False, + ): self.__dict__.update(locals()) self.mean = None self.std = None self.principal_components = None - def flow(self, X, y, batch_size=32, shuffle=False, seed=None, save_to_dir=None, save_prefix="", save_format="jpeg"): if seed: random.seed(seed) @@ -175,7 +172,6 @@ def flow(self, X, y, batch_size=32, shuffle=False, seed=None, save_to_dir=None, yield bX, y[b*batch_size:b*batch_size+nb_samples] - def standardize(self, x): if self.featurewise_center: x -= self.mean @@ -194,7 +190,6 @@ def standardize(self, x): return x - def random_transform(self, x): if self.rotation_range: x = random_rotation(x, self.rotation_range) diff --git a/keras/preprocessing/sequence.py b/keras/preprocessing/sequence.py index ce9bc76a7dfd..3465e10b241d 100644 --- a/keras/preprocessing/sequence.py +++ b/keras/preprocessing/sequence.py @@ -135,7 +135,7 @@ def skipgrams(sequence, vocabulary_size, words = [c[0] for c in couples] random.shuffle(words) - couples += [[words[i%len(words)], random.randint(1, vocabulary_size-1)] for i in range(nb_negative_samples)] + couples += [[words[i %len(words)], random.randint(1, vocabulary_size-1)] for i in range(nb_negative_samples)] if categorical: labels += [[1,0]]*nb_negative_samples else: diff --git a/keras/preprocessing/text.py b/keras/preprocessing/text.py index 83d693bfc680..34121390bd1c 100644 --- a/keras/preprocessing/text.py +++ b/keras/preprocessing/text.py @@ -5,7 +5,8 @@ ''' from __future__ import absolute_import -import string, sys +import string +import sys import numpy as np from six.moves import range from six.moves import zip diff --git a/pytest.ini b/pytest.ini index faf7da6fbf05..49c56c3449ac 100644 --- a/pytest.ini +++ b/pytest.ini @@ -5,3 +5,34 @@ addopts=-v --durations=10 --cov-report term-missing --cov=keras + +# Do not run tests in the build folder +norecursedirs= build + +# PEP-8 The following are ignored: +# E251 unexpected spaces around keyword / parameter equals +# E225 missing whitespace around operator +# E226 missing whitespace around arithmetic operator +# W291 trailing whitespace +# W293 blank line contains whitespace +# E501 line too long (82 > 79 characters) +# E402 module level import not at top of file - temporary measure to coninue adding ros python packaged in sys.path +# E731 do not assign a lambda expression, use a def +# E302 two blank lines between the functions +# E231 missing whitespace after , +# E241 multiple spaces after ',' +# E261 at least two spaces before inline comment + + +pep8ignore=* E251 \ + * E225 \ + * E226 \ + * W291 \ + * W293 \ + * E501 \ + * E402 \ + * E731 \ + * E302 \ + * E231 \ + * E241 \ + * E261 diff --git a/tests/integration_tests/test_temporal_data_tasks.py b/tests/integration_tests/test_temporal_data_tasks.py index 5acc21853064..95dff126eef4 100644 --- a/tests/integration_tests/test_temporal_data_tasks.py +++ b/tests/integration_tests/test_temporal_data_tasks.py @@ -48,7 +48,7 @@ def test_temporal_regression(): classification=False) model = Sequential() model.add(GRU(y_train.shape[-1], - input_shape=(X_train.shape[1], X_train.shape[2]))) + input_shape=(X_train.shape[1], X_train.shape[2]))) model.compile(loss='hinge', optimizer='adam') history = model.fit(X_train, y_train, nb_epoch=5, batch_size=16, validation_data=(X_test, y_test), verbose=0) @@ -71,7 +71,7 @@ def test_sequence_to_sequence(): model = Sequential() model.add(TimeDistributedDense(y_train.shape[-1], - input_shape=(X_train.shape[1], X_train.shape[2]))) + input_shape=(X_train.shape[1], X_train.shape[2]))) model.compile(loss='hinge', optimizer='rmsprop') history = model.fit(X_train, y_train, nb_epoch=20, batch_size=16, validation_data=(X_test, y_test), verbose=0) diff --git a/tests/keras/layers/test_convolutional.py b/tests/keras/layers/test_convolutional.py index 0623dd6fb165..819ebcbdebea 100644 --- a/tests/keras/layers/test_convolutional.py +++ b/tests/keras/layers/test_convolutional.py @@ -188,7 +188,6 @@ def test_upsampling_2d(): input_nb_row = 11 input_nb_col = 12 - for dim_ordering in ['th', 'tf']: if dim_ordering == 'th': input = np.random.rand(nb_samples, stack_size, input_nb_row, diff --git a/tests/keras/layers/test_core.py b/tests/keras/layers/test_core.py index 621626a01e7e..17d523035dfc 100644 --- a/tests/keras/layers/test_core.py +++ b/tests/keras/layers/test_core.py @@ -153,7 +153,7 @@ def test_sequences(): layer = core.Masking() func = K.function([layer.input], [layer.get_output_mask()]) input_data = np.array([[[1], [2], [3], [0]], - [[0], [4], [5], [0]]], dtype=np.int32) + [[0], [4], [5], [0]]], dtype=np.int32) # This is the expected output mask, one dimension less expected = np.array([[1, 1, 1, 0], [0, 1, 1, 0]]) @@ -170,7 +170,7 @@ def test_non_zero(): layer = core.Masking(5) func = K.function([layer.input], [layer.get_output_mask()]) input_data = np.array([[[1, 1], [2, 1], [3, 1], [5, 5]], - [[1, 5], [5, 0], [0, 0], [0, 0]]], + [[1, 5], [5, 0], [0, 0], [0, 0]]], dtype=np.int32) output = func([input_data])[0] expected = np.array([[1, 1, 1, 0], [1, 1, 1, 1]]) @@ -185,11 +185,11 @@ def test_non_zero_output(): func = K.function([layer.input], [layer.get_output()]) input_data = np.array([[[1, 1], [2, 1], [3, 1], [5, 5]], - [[1, 5], [5, 0], [0, 0], [0, 0]]], + [[1, 5], [5, 0], [0, 0], [0, 0]]], dtype=np.int32) output = func([input_data])[0] expected = np.array([[[1, 1], [2, 1], [3, 1], [0, 0]], - [[1, 5], [5, 0], [0, 0], [0, 0]]]) + [[1, 5], [5, 0], [0, 0], [0, 0]]]) assert np.all(output == expected), 'Output not as expected' diff --git a/tests/keras/layers/test_recurrent.py b/tests/keras/layers/test_recurrent.py index cc4801b76015..d4e12bd3bae1 100644 --- a/tests/keras/layers/test_recurrent.py +++ b/tests/keras/layers/test_recurrent.py @@ -65,7 +65,6 @@ def _runner(layer_class): assert(out4.max() != out5.max()) - def test_SimpleRNN(): _runner(recurrent.SimpleRNN) diff --git a/tests/keras/preprocessing/test_image.py b/tests/keras/preprocessing/test_image.py index dc01d137ead5..7f9dafb23f5c 100644 --- a/tests/keras/preprocessing/test_image.py +++ b/tests/keras/preprocessing/test_image.py @@ -55,7 +55,7 @@ def test_image_data_generator(): for x,y in generator.flow(images,np.arange(images.shape[0]), shuffle=True, save_to_dir='test_images'): assert x.shape[1:] == images.shape[1:] - #TODO: make sure the normalization is working as inteded + # TODO: make sure the normalization is working as inteded if __name__ == '__main__': diff --git a/tests/keras/test_models.py b/tests/keras/test_models.py index 7f1a75d483be..fba4d7c0598c 100644 --- a/tests/keras/test_models.py +++ b/tests/keras/test_models.py @@ -357,6 +357,7 @@ def test_merge_overlap(): def test_lambda(): (X_train, y_train), (X_test, y_test) = _get_test_data() + def func(X): s = X[0] for i in range(1, len(X)):