Type check with pytype (#565)

* Configure pytype in CI

* Fix pytype types

* gitignore: ignore pytype cache

* Update Docker

* fix stray import error

* PEP 561 compliance

* Update changelog

* [Temporary] Explicitly install pytype

* Simplify Dockerfile

* Set appropriate python_version for pytype

* Update Docker

* Lint

* Update Docker tags

* Fix type error (mujoco_py not available in Docker)

* [Temporary] Use my Docker image

* Fix cmd_util after Gym upgrade

* Use new JRE so that Codacy Coverage uploader can function

* Update Docker tags

* Change return type to float

* try-except for backward compatibility

* add error message

* Update Docker tag for Travis

* fix type errors

* Update Docker install instructions

* Document type checking

* Do Codacy uploads from PR build (unless in fork)

Author: AdamGleave

.dockerignore

@@ -0,0 +1 @@
+.gitignore

.github/PULL_REQUEST_TEMPLATE.md

@@ -24,5 +24,6 @@
 - [ ] My change requires a change to the documentation.
 - [ ] I have updated the tests accordingly (*required for a bug fix or a new feature*).
 - [ ] I have updated the documentation accordingly.
+- [ ] I have ensured `pytest` and `pytype` both pass.
 
 <!--- This Template is an edited version of the one from https://github.com/evilsocket/pwnagotchi/ -->

.gitignore

@@ -4,6 +4,7 @@
 *.py~
 *.bak
 .pytest_cache
+.pytype
 .DS_Store
 .idea
 .coverage
@@ -19,13 +20,11 @@ keys/
 
 # Virtualenv
 /env
-
+/venv
 
 *.sublime-project
 *.sublime-workspace
 
-.idea
-
 logs/
 
 .ipynb_checkpoints

.travis.yml

@@ -4,7 +4,7 @@ python:
 
 env:
   global:
-    - DOCKER_IMAGE=araffin/stable-baselines-cpu:v2.7.1
+    - DOCKER_IMAGE=stablebaselines/stable-baselines-cpu:v2.9.0
 
 notifications:
   email: false
@@ -41,8 +41,12 @@ jobs:
       script:
         - 'docker run -it --rm --mount src=$(pwd),target=/root/code/stable-baselines,type=bind ${DOCKER_IMAGE} bash -c "cd /root/code/stable-baselines/ && pip install .[docs] && pushd docs/ && make clean && make html"'
 
+    - name: "Type Checking"
+      script:
+        - 'docker run --rm --mount src=$(pwd),target=/root/code/stable-baselines,type=bind ${DOCKER_IMAGE} bash -c "cd /root/code/stable-baselines/ && pytype"'
+
     - stage: Codacy Trigger
       if: type != pull_request
       script:
         # When all test coverage reports have been uploaded, instruct Codacy to start analysis.
-        - 'docker run -it --rm --network host --ipc=host --mount src=$(pwd),target=/root/code/stable-baselines,type=bind --env CODACY_PROJECT_TOKEN=${CODACY_PROJECT_TOKEN} ${DOCKER_IMAGE} bash -c "cd /root/code/stable-baselines/ && /root/code/codacy-coverage-reporter final"'
+        - 'docker run -it --rm --network host --ipc=host --mount src=$(pwd),target=/root/code/stable-baselines,type=bind --env CODACY_PROJECT_TOKEN=${CODACY_PROJECT_TOKEN} ${DOCKER_IMAGE} bash -c "cd /root/code/stable-baselines/ && java -jar /root/code/codacy-coverage-reporter.jar final"'

CONTRIBUTING.md

@@ -88,7 +88,7 @@ All new features must add tests in the `tests/` folder ensuring that everything
 We use [pytest](https://pytest.org/).
 Also, when a bug fix is proposed, tests should be added to avoid regression.
 
-To run tests:
+To run tests with `pytest` and type checking with `pytype`:
 
 ```
 ./scripts/run_tests.sh

Dockerfile

@@ -0,0 +1,52 @@
+ARG PARENT_IMAGE
+ARG USE_GPU
+FROM $PARENT_IMAGE
+
+RUN apt-get -y update \
+    && apt-get -y install \
+    curl \
+    cmake \
+    default-jre \
+    git \
+    jq \
+    python-dev \
+    python-pip \
+    python3-dev \
+    libfontconfig1 \
+    libglib2.0-0 \
+    libsm6 \
+    libxext6 \
+    libxrender1 \
+    libopenmpi-dev \
+    zlib1g-dev \
+    && apt-get clean \
+    && rm -rf /var/lib/apt/lists/*
+
+ENV CODE_DIR /root/code
+ENV VENV /root/venv
+
+COPY ./setup.py /root/code/setup.py
+RUN \
+    mkdir -p ${CODE_DIR}/stable_baselines && \
+    pip install virtualenv && \
+    virtualenv $VENV --python=python3 && \
+    . $VENV/bin/activate && \
+    cd $CODE_DIR && \
+    pip install --upgrade pip && \
+    if [[ $USE_GPU == "True" ]]; then \
+        TENSORFLOW_PACKAGE="tensorflow-gpu==1.8.0"; \
+    else \
+        TENSORFLOW_PACKAGE="tensorflow==1.8.0"; \
+    fi; \
+    pip install ${TENSORFLOW_PACKAGE} && \
+    pip install -e .[mpi,tests] && \
+    pip install codacy-coverage && \
+    rm -rf $HOME/.cache/pip
+
+ENV PATH=$VENV/bin:$PATH
+
+# Codacy code coverage report: used for partial code coverage reporting
+RUN cd $CODE_DIR && \
+    curl -Ls -o codacy-coverage-reporter.jar "$(curl -Ls https://api.github.com/repos/codacy/codacy-coverage-reporter/releases/latest | jq -r '.assets | map({name, browser_download_url} | select(.name | (contains("codacy-coverage-reporter") and endswith("assembly.jar")))) | .[0].browser_download_url')"
+
+CMD /bin/bash

docker/Dockerfile.cpu

@@ -101,13 +101,13 @@ GPU image (requires `nvidia-docker`_):
 
 .. code-block:: bash
 
-   docker pull araffin/stable-baselines
+   docker pull stablebaselines/stable-baselines
 
 CPU only:
 
 .. code-block:: bash
 
-   docker pull araffin/stable-baselines-cpu
+   docker pull stablebaselines/stable-baselines-cpu
 
 Build the Docker Images
 ~~~~~~~~~~~~~~~~~~~~~~~~
@@ -116,13 +116,13 @@ Build GPU image (with nvidia-docker):
 
 .. code-block:: bash
 
-   docker build . -f docker/Dockerfile.gpu -t stable-baselines
+   USE_GPU=True ./scripts/build_docker.sh
 
 Build CPU image:
 
 .. code-block:: bash
 
-   docker build . -f docker/Dockerfile.cpu -t stable-baselines-cpu
+   ./scripts/build_docker.sh
 
 Note: if you are using a proxy, you need to pass extra params during
 build and do some `tweaks`_:
@@ -138,7 +138,7 @@ Run the nvidia-docker GPU image
 
 .. code-block:: bash
 
-   docker run -it --runtime=nvidia --rm --network host --ipc=host --name test --mount src="$(pwd)",target=/root/code/stable-baselines,type=bind araffin/stable-baselines bash -c 'cd /root/code/stable-baselines/ && pytest tests/'
+   docker run -it --runtime=nvidia --rm --network host --ipc=host --name test --mount src="$(pwd)",target=/root/code/stable-baselines,type=bind stablebaselines/stable-baselines bash -c 'cd /root/code/stable-baselines/ && pytest tests/'
 
 Or, with the shell file:
 
@@ -150,7 +150,7 @@ Run the docker CPU image
 
 .. code-block:: bash
 
-   docker run -it --rm --network host --ipc=host --name test --mount src="$(pwd)",target=/root/code/stable-baselines,type=bind araffin/stable-baselines-cpu bash -c 'cd /root/code/stable-baselines/ && pytest tests/'
+   docker run -it --rm --network host --ipc=host --name test --mount src="$(pwd)",target=/root/code/stable-baselines,type=bind stablebaselines/stable-baselines-cpu bash -c 'cd /root/code/stable-baselines/ && pytest tests/'
 
 Or, with the shell file:
 

docker/Dockerfile.gpu

@@ -19,12 +19,15 @@ New Features:
 - Add `n_cpu_tf_sess` to model constructor to choose the number of threads used by Tensorflow
 - `VecNormalize` now supports being pickled and unpickled.
 - Add parameter `exploration_initial_eps` to DQN. (@jdossgollin)
+- Add type checking and PEP 561 compliance.
+  Note: most functions are still not annotated, this will be a gradual process.
 
 Bug Fixes:
 ^^^^^^^^^^
 - Fix seeding, so it is now possible to have deterministic results on cpu
 - Fix a bug in DDPG where `predict` method with `deterministic=False` would fail
 - Fix a bug in TRPO: mean_losses was not initialized causing the logger to crash when there was no gradients (@MarvineGothic)
+- Fix a bug in `cmd_util` from API change in recent Gym versions
 
 Deprecations:
 ^^^^^^^^^^^^^

docs/guide/install.rst

@@ -0,0 +1,17 @@
+#!/bin/bash
+
+CPU_PARENT=ubuntu:16.04
+GPU_PARENT=nvidia/cuda:9.0-cudnn7-runtime-ubuntu16.04
+
+TAG=stablebaselines/stable-baselines
+VERSION=v2.9.0
+
+if [[ ${USE_GPU} == "True" ]]; then
+  PARENT=${GPU_PARENT}
+else
+  PARENT=${CPU_PARENT}
+  TAG="${TAG}-cpu"
+fi
+
+docker build --build-arg PARENT_IMAGE=${PARENT} -t ${TAG}:${VERSION} .
+

docs/misc/changelog.rst

@@ -6,7 +6,6 @@ cmd_line="$@"
 echo "Executing in the docker (cpu image):"
 echo $cmd_line
 
-
 docker run -it --rm --network host --ipc=host \
- --mount src=$(pwd),target=/root/code/stable-baselines,type=bind araffin/stable-baselines-cpu:v2.7.1 \
+ --mount src=$(pwd),target=/root/code/stable-baselines,type=bind stablebaselines/stable-baselines-cpu:v2.9.0 \
   bash -c "cd /root/code/stable-baselines/ && $cmd_line"

scripts/build_docker.sh

@@ -8,5 +8,5 @@ echo $cmd_line
 
 
 docker run -it --runtime=nvidia --rm --network host --ipc=host \
-  --mount src=$(pwd),target=/root/code/stable-baselines,type=bind araffin/stable-baselines:v2.7.1 \
+  --mount src=$(pwd),target=/root/code/stable-baselines,type=bind stablebaselines/stable-baselines:v2.9.0 \
   bash -c "cd /root/code/stable-baselines/ && $cmd_line"

scripts/run_docker_cpu.sh

@@ -1,2 +1,3 @@
 #!/bin/bash
 python -m pytest --cov-config .coveragerc --cov-report html --cov-report term --cov=. -v
+pytype

scripts/run_docker_gpu.sh

@@ -18,18 +18,15 @@ TEST_GLOB=$1
 set -e  # exit immediately on any error
 
 # For pull requests from fork, Codacy token is not available, leading to build failure
-if [ "$TRAVIS_PULL_REQUEST" != "false" ]; then
+if [[ ${CODACY_PROJECT_TOKEN} = "" ]]; then
+  echo "WARNING: CODACY_PROJECT_TOKEN not set. Skipping Codacy upload."
+  echo "(This is normal when building in a fork and can be ignored.)"
   ${DOCKER_CMD} ${DOCKER_IMAGE} \
       bash -c "${BASH_CMD} && \
                pytest --cov-config .coveragerc --cov-report term --cov=. -v tests/test_${TEST_GLOB}"
 else
-  if [[ ${CODACY_PROJECT_TOKEN} = "" ]]; then
-    echo "Need CODACY_PROJECT_TOKEN environment variable to be set."
-    exit 1
-  fi
-
   ${DOCKER_CMD} --env CODACY_PROJECT_TOKEN=${CODACY_PROJECT_TOKEN} ${DOCKER_IMAGE} \
       bash -c "${BASH_CMD} && \
                 pytest --cov-config .coveragerc --cov-report term --cov-report xml --cov=. -v tests/test_${TEST_GLOB} && \
-                /root/code/codacy-coverage-reporter report -l python -r coverage.xml --partial"
+                java -jar /root/code/codacy-coverage-reporter.jar report -l python -r coverage.xml --partial"
 fi

scripts/run_tests.sh

@@ -16,3 +16,7 @@ filterwarnings =
 		# Gym warnings
 		ignore:Parameters to load are deprecated.:DeprecationWarning
 		ignore:the imp module is deprecated in favour of importlib:PendingDeprecationWarning
+
+[pytype]
+inputs = stable_baselines
+python_version = 3.5

scripts/run_tests_travis.sh

@@ -105,6 +105,9 @@
 setup(name='stable_baselines',
       packages=[package for package in find_packages()
                 if package.startswith('stable_baselines')],
+      package_data={
+          'stable_baselines': ['py.typed'],
+      },
       install_requires=[
           'gym[atari,classic_control]>=0.10.9',
           'scipy',
@@ -124,6 +127,7 @@
             'pytest-cov',
             'pytest-env',
             'pytest-xdist',
+            'pytype',
         ],
         'docs': [
             'sphinx',

setup.cfg

@@ -1,4 +1,5 @@
 import time
+import warnings
 
 import numpy as np
 import tensorflow as tf
@@ -538,7 +539,9 @@ def learn(self, total_timesteps, callback=None, log_interval=100, tb_log_name="A
                         logger.record_tabular(name, float(val))
                     logger.dump_tabular()
 
-                if self.replay_ratio > 0 and buffer.has_atleast(self.replay_start):
+                if (self.replay_ratio > 0 and
+                    buffer is not None and
+                    buffer.has_atleast(self.replay_start)):
                     samples_number = np.random.poisson(self.replay_ratio)
                     for _ in range(samples_number):
                         # get obs, actions, rewards, mus, dones from buffer.

setup.py

@@ -337,12 +337,14 @@ def learn(self, total_timesteps, callback=None, log_interval=100, tb_log_name="A
             ep_info_buf = deque(maxlen=100)
 
             for update in range(1, total_timesteps // self.n_batch + 1):
+                # pytype:disable=bad-unpacking
                 # true_reward is the reward without discount
                 if isinstance(runner, PPO2Runner):
                     # We are using GAE
                     obs, returns, masks, actions, values, _, states, ep_infos, true_reward = runner.run()
                 else:
                     obs, states, returns, masks, actions, values, ep_infos, true_reward = runner.run()
+                # pytype:enable=bad-unpacking
 
                 ep_info_buf.extend(ep_infos)
                 policy_loss, value_loss, policy_entropy = self._train_step(obs, states, returns, masks, actions, values,