diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index 761e41e..2b35d61 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -24,7 +24,11 @@ - [ ] 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 checked the codestyle using `make lint` -- [ ] I have ensured `make pytest` and `make type` both pass. +- [ ] I have reformatted the code using `make format` (**required**) +- [ ] I have checked the codestyle using `make check-codestyle` and `make lint` (**required**) +- [ ] I have ensured `make pytest` and `make type` both pass. (**required**) + + +Note: we are using a maximum length of 127 characters per line diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index cbad4a1..47ca919 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -38,6 +38,9 @@ jobs: - name: Type check run: | make type + - name: Check codestyle + run: | + make check-codestyle - name: Lint with flake8 run: | make lint diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 8eeaf41..695c145 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -1,4 +1,4 @@ -image: stablebaselines/stable-baselines3-cpu:0.8.0a1 +image: stablebaselines/stable-baselines3-cpu:0.8.0a4 type-check: script: @@ -15,4 +15,5 @@ doc-build: lint-check: script: + - make check-codestyle - make lint diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index ff56c13..e04e59e 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -38,23 +38,9 @@ pip install -e .[docs,tests,extra] ## Codestyle -We follow the [PEP8 codestyle](https://www.python.org/dev/peps/pep-0008/). Please order the imports as follows: +We are using [black codestyle](https://github.com/psf/black) (max line length of 127 characters) together with [isort](https://github.com/timothycrosley/isort) to sort the imports. -1. built-in -2. packages -3. current module - -with one space between each, that gives for instance: -```python -import os -import warnings - -import numpy as np - -from stable_baselines3 import PPO -``` - -In general, we recommend using pycharm to format everything in an efficient way. +**Please run `make format`** to reformat your code. You can check the codestyle using `make check-codestyle` and `make lint`. Please document each function/method and [type](https://google.github.io/pytype/user_guide.html) them using the following template: @@ -77,11 +63,10 @@ def my_function(arg1: type1, arg2: type2) -> returntype: Before proposing a PR, please open an issue, where the feature will be discussed. This prevent from duplicated PR to be proposed and also ease the code review process. Each PR need to be reviewed and accepted by at least one of the maintainers (@hill-a, @araffin, @erniejunior, @AdamGleave or @Miffyli). -A PR must pass the Continuous Integration tests (travis + codacy) to be merged with the master branch. +A PR must pass the Continuous Integration tests to be merged with the master branch. -Note: in rare cases, we can create exception for codacy failure. -## Test +## Tests All new features must add tests in the `tests/` folder ensuring that everything works fine. We use [pytest](https://pytest.org/). @@ -99,12 +84,18 @@ Type checking with `pytype`: make type ``` -Codestyle check with `flake8`: +Codestyle check with `black`, `isort` and `flake8`: ``` +make check-codestyle make lint ``` +To run `pytype`, `format` and `lint` in one command: +``` +make commit-checks +``` + Build the documentation: ``` @@ -121,6 +112,7 @@ make spelling ## Changelog and Documentation Please do not forget to update the changelog (`docs/misc/changelog.rst`) and add documentation if needed. +You should add your username next to each changelog entry that you added. If this is your first contribution, please add your username at the bottom too. A README is present in the `docs/` folder for instructions on how to build the documentation. diff --git a/Dockerfile b/Dockerfile index 0b30e6d..2ed1d23 100644 --- a/Dockerfile +++ b/Dockerfile @@ -10,7 +10,8 @@ RUN apt-get update && apt-get install -y --no-install-recommends \ curl \ ca-certificates \ libjpeg-dev \ - libpng-dev && \ + libpng-dev \ + libglib2.0-0 && \ rm -rf /var/lib/apt/lists/* # Install anaconda abd dependencies @@ -37,15 +38,4 @@ RUN \ pip install opencv-python-headless && \ rm -rf $HOME/.cache/pip - -# Codacy deps -RUN apt-get update && apt-get install -y --no-install-recommends \ - default-jre \ - jq && \ - rm -rf /var/lib/apt/lists/* - -# 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 | (startswith("codacy-coverage-reporter") and contains("assembly") and endswith(".jar")))) | .[0].browser_download_url')" - CMD /bin/bash diff --git a/Makefile b/Makefile index 05ce4d2..749bc02 100644 --- a/Makefile +++ b/Makefile @@ -14,6 +14,20 @@ lint: # exit-zero treats all errors as warnings. flake8 ${LINT_PATHS} --count --exit-zero --statistics +format: + # Sort imports + isort ${LINT_PATHS} + # Reformat using black + black -l 127 ${LINT_PATHS} + +check-codestyle: + # Sort imports + isort --check ${LINT_PATHS} + # Reformat using black + black --check -l 127 ${LINT_PATHS} + +commit-checks: format type lint + doc: cd docs && make html @@ -23,8 +37,6 @@ spelling: clean: cd docs && make clean -.PHONY: clean spelling doc lint - # Build docker images # If you do export RELEASE=True, it will also push them docker: docker-cpu docker-gpu @@ -46,3 +58,5 @@ test-release: python setup.py sdist python setup.py bdist_wheel twine upload --repository-url https://test.pypi.org/legacy/ dist/* + +.PHONY: clean spelling doc lint format check-codestyle commit-checks diff --git a/README.md b/README.md index d6338af..95e7c15 100644 --- a/README.md +++ b/README.md @@ -1,7 +1,7 @@ [![pipeline status](https://gitlab.com/araffin/stable-baselines3/badges/master/pipeline.svg)](https://gitlab.com/araffin/stable-baselines3/-/commits/master) [![Documentation Status](https://readthedocs.org/projects/stable-baselines/badge/?version=master)](https://stable-baselines3.readthedocs.io/en/master/?badge=master) [![coverage report](https://gitlab.com/araffin/stable-baselines3/badges/master/coverage.svg)](https://gitlab.com/araffin/stable-baselines3/-/commits/master) - +[![codestyle](https://img.shields.io/badge/code%20style-black-000000.svg)](https://github.com/psf/black) **WARNING: Stable Baselines3 is currently in a beta version, breaking changes may occur before 1.0 is released** diff --git a/docs/conf.py b/docs/conf.py index 78138f6..320834c 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -20,12 +20,13 @@ from unittest.mock import MagicMock # PyEnchant. try: import sphinxcontrib.spelling # noqa: F401 + enable_spell_check = True except ImportError: enable_spell_check = False # source code directory, relative to this file, for sphinx-autobuild -sys.path.insert(0, os.path.abspath('..')) +sys.path.insert(0, os.path.abspath("..")) class Mock(MagicMock): @@ -44,18 +45,18 @@ MOCK_MODULES = [] sys.modules.update((mod_name, Mock()) for mod_name in MOCK_MODULES) # Read version from file -version_file = os.path.join(os.path.dirname(__file__), '../stable_baselines3', 'version.txt') -with open(version_file, 'r') as file_handler: +version_file = os.path.join(os.path.dirname(__file__), "../stable_baselines3", "version.txt") +with open(version_file, "r") as file_handler: __version__ = file_handler.read().strip() # -- Project information ----------------------------------------------------- -project = 'Stable Baselines3' -copyright = '2020, Stable Baselines3' -author = 'Stable Baselines3 Contributors' +project = "Stable Baselines3" +copyright = "2020, Stable Baselines3" +author = "Stable Baselines3 Contributors" # The short X.Y version -version = 'master (' + __version__ + ' )' +version = "master (" + __version__ + " )" # The full version, including alpha/beta/rc tags release = __version__ @@ -70,30 +71,30 @@ release = __version__ # extensions coming with Sphinx (named 'sphinx.ext.*') or your custom # ones. extensions = [ - 'sphinx.ext.autodoc', + "sphinx.ext.autodoc", # 'sphinx_autodoc_typehints', - 'sphinx.ext.autosummary', - 'sphinx.ext.mathjax', - 'sphinx.ext.ifconfig', - 'sphinx.ext.viewcode', + "sphinx.ext.autosummary", + "sphinx.ext.mathjax", + "sphinx.ext.ifconfig", + "sphinx.ext.viewcode", # 'sphinx.ext.intersphinx', # 'sphinx.ext.doctest' ] if enable_spell_check: - extensions.append('sphinxcontrib.spelling') + extensions.append("sphinxcontrib.spelling") # Add any paths that contain templates here, relative to this directory. -templates_path = ['_templates'] +templates_path = ["_templates"] # The suffix(es) of source filenames. # You can specify multiple suffix as a list of string: # # source_suffix = ['.rst', '.md'] -source_suffix = '.rst' +source_suffix = ".rst" # The master toctree document. -master_doc = 'index' +master_doc = "index" # The language for content autogenerated by Sphinx. Refer to documentation # for a list of supported languages. @@ -105,10 +106,10 @@ language = None # List of patterns, relative to source directory, that match files and # directories to ignore when looking for source files. # This pattern also affects html_static_path and html_extra_path . -exclude_patterns = ['_build', 'Thumbs.db', '.DS_Store'] +exclude_patterns = ["_build", "Thumbs.db", ".DS_Store"] # The name of the Pygments (syntax highlighting) style to use. -pygments_style = 'sphinx' +pygments_style = "sphinx" # -- Options for HTML output ------------------------------------------------- @@ -117,13 +118,13 @@ pygments_style = 'sphinx' # a list of builtin themes. # Fix for read the docs -on_rtd = os.environ.get('READTHEDOCS') == 'True' +on_rtd = os.environ.get("READTHEDOCS") == "True" if on_rtd: - html_theme = 'default' + html_theme = "default" else: - html_theme = 'sphinx_rtd_theme' + html_theme = "sphinx_rtd_theme" -html_logo = '_static/img/logo.png' +html_logo = "_static/img/logo.png" def setup(app): @@ -139,7 +140,7 @@ def setup(app): # Add any paths that contain custom static files (such as style sheets) here, # relative to this directory. They are copied after the builtin static files, # so a file named "default.css" will overwrite the builtin "default.css". -html_static_path = ['_static'] +html_static_path = ["_static"] # Custom sidebar templates, must be a dictionary that maps document names # to template names. @@ -155,7 +156,7 @@ html_static_path = ['_static'] # -- Options for HTMLHelp output --------------------------------------------- # Output file base name for HTML help builder. -htmlhelp_basename = 'StableBaselines3doc' +htmlhelp_basename = "StableBaselines3doc" # -- Options for LaTeX output ------------------------------------------------ @@ -164,15 +165,12 @@ latex_elements = { # The paper size ('letterpaper' or 'a4paper'). # # 'papersize': 'letterpaper', - # The font size ('10pt', '11pt' or '12pt'). # # 'pointsize': '10pt', - # Additional stuff for the LaTeX preamble. # # 'preamble': '', - # Latex figure (float) alignment # # 'figure_align': 'htbp', @@ -182,8 +180,7 @@ latex_elements = { # (source start file, target name, title, # author, documentclass [howto, manual, or own class]). latex_documents = [ - (master_doc, 'StableBaselines3.tex', 'Stable Baselines3 Documentation', - 'Stable Baselines3 Contributors', 'manual'), + (master_doc, "StableBaselines3.tex", "Stable Baselines3 Documentation", "Stable Baselines3 Contributors", "manual"), ] @@ -191,10 +188,7 @@ latex_documents = [ # One entry per manual page. List of tuples # (source start file, name, description, authors, manual section). -man_pages = [ - (master_doc, 'stablebaselines3', 'Stable Baselines3 Documentation', - [author], 1) -] +man_pages = [(master_doc, "stablebaselines3", "Stable Baselines3 Documentation", [author], 1)] # -- Options for Texinfo output ---------------------------------------------- @@ -203,9 +197,15 @@ man_pages = [ # (source start file, target name, title, author, # dir menu entry, description, category) texinfo_documents = [ - (master_doc, 'StableBaselines3', 'Stable Baselines3 Documentation', - author, 'StableBaselines3', 'One line description of project.', - 'Miscellaneous'), + ( + master_doc, + "StableBaselines3", + "Stable Baselines3 Documentation", + author, + "StableBaselines3", + "One line description of project.", + "Miscellaneous", + ), ] diff --git a/docs/misc/changelog.rst b/docs/misc/changelog.rst index 513b2b3..0fa130a 100644 --- a/docs/misc/changelog.rst +++ b/docs/misc/changelog.rst @@ -41,8 +41,10 @@ Others: - Split the ``collect_rollout()`` method for off-policy algorithms - Added ``_on_step()`` for off-policy base class - Optimized replay buffer size by removing the need of ``next_observations`` numpy array +- Switch to ``black`` codestyle and added ``make format``, ``make check-codestyle`` and ``commit-checks`` - Ignored errors from newer pytype version - Added a check when using ``gSDE`` +- Removed codacy dependency from Dockerfile Documentation: ^^^^^^^^^^^^^^ diff --git a/setup.cfg b/setup.cfg index 1d01700..011c3d9 100644 --- a/setup.cfg +++ b/setup.cfg @@ -21,7 +21,7 @@ filterwarnings = inputs = stable_baselines3 [flake8] -ignore = W503,W504 # line breaks before and after binary operators +ignore = W503,W504,E203,E231 # line breaks before and after binary operators # Ignore import not used when aliases are defined per-file-ignores = ./stable_baselines3/__init__.py:F401 @@ -48,3 +48,8 @@ exclude = max-complexity = 15 # The GitHub editor is 127 chars wide max-line-length = 127 + +[isort] +profile = black +line_length = 127 +src_paths = stable_baselines3 diff --git a/setup.py b/setup.py index 5733441..8401c7c 100644 --- a/setup.py +++ b/setup.py @@ -1,7 +1,8 @@ import os -from setuptools import setup, find_packages -with open(os.path.join('stable_baselines3', 'version.txt'), 'r') as file_handler: +from setuptools import find_packages, setup + +with open(os.path.join("stable_baselines3", "version.txt"), "r") as file_handler: __version__ = file_handler.read().strip() @@ -64,66 +65,69 @@ model = PPO('MlpPolicy', 'CartPole-v1').learn(10000) """ # noqa:E501 -setup(name='stable_baselines3', - packages=[package for package in find_packages() - if package.startswith('stable_baselines3')], - package_data={ - 'stable_baselines3': ['py.typed', 'version.txt'] - }, - install_requires=[ - 'gym>=0.17', - 'numpy', - 'torch>=1.4.0', - # For saving models - 'cloudpickle', - # For reading logs - 'pandas', - # Plotting learning curves - 'matplotlib' - ], - extras_require={ - 'tests': [ - # Run tests and coverage - 'pytest', - 'pytest-cov', - 'pytest-env', - 'pytest-xdist', - # Type check - 'pytype', - # Lint code - 'flake8>=3.8' - ], - 'docs': [ - 'sphinx', - 'sphinx-autobuild', - 'sphinx-rtd-theme', - # For spelling - 'sphinxcontrib.spelling', - # Type hints support - # 'sphinx-autodoc-typehints' - ], - 'extra': [ - # For render - 'opencv-python', - # For atari games, - 'atari_py~=0.2.0', 'pillow', - # Tensorboard support - 'tensorboard', - # Checking memory taken by replay buffer - 'psutil' - ] - }, - description='Pytorch version of Stable Baselines, implementations of reinforcement learning algorithms.', - author='Antonin Raffin', - url='https://github.com/DLR-RM/stable-baselines3', - author_email='antonin.raffin@dlr.de', - keywords="reinforcement-learning-algorithms reinforcement-learning machine-learning " - "gym openai stable baselines toolbox python data-science", - license="MIT", - long_description=long_description, - long_description_content_type='text/markdown', - version=__version__, - ) +setup( + name="stable_baselines3", + packages=[package for package in find_packages() if package.startswith("stable_baselines3")], + package_data={"stable_baselines3": ["py.typed", "version.txt"]}, + install_requires=[ + "gym>=0.17", + "numpy", + "torch>=1.4.0", + # For saving models + "cloudpickle", + # For reading logs + "pandas", + # Plotting learning curves + "matplotlib", + ], + extras_require={ + "tests": [ + # Run tests and coverage + "pytest", + "pytest-cov", + "pytest-env", + "pytest-xdist", + # Type check + "pytype", + # Lint code + "flake8>=3.8", + # Sort imports + "isort>=5.0", + # Reformat + "black", + ], + "docs": [ + "sphinx", + "sphinx-autobuild", + "sphinx-rtd-theme", + # For spelling + "sphinxcontrib.spelling", + # Type hints support + # 'sphinx-autodoc-typehints' + ], + "extra": [ + # For render + "opencv-python", + # For atari games, + "atari_py~=0.2.0", + "pillow", + # Tensorboard support + "tensorboard", + # Checking memory taken by replay buffer + "psutil", + ], + }, + description="Pytorch version of Stable Baselines, implementations of reinforcement learning algorithms.", + author="Antonin Raffin", + url="https://github.com/DLR-RM/stable-baselines3", + author_email="antonin.raffin@dlr.de", + keywords="reinforcement-learning-algorithms reinforcement-learning machine-learning " + "gym openai stable baselines toolbox python data-science", + license="MIT", + long_description=long_description, + long_description_content_type="text/markdown", + version=__version__, +) # python setup.py sdist # python setup.py bdist_wheel diff --git a/stable_baselines3/common/base_class.py b/stable_baselines3/common/base_class.py index 20091de..1ba7cc0 100644 --- a/stable_baselines3/common/base_class.py +++ b/stable_baselines3/common/base_class.py @@ -158,7 +158,7 @@ class BaseAlgorithm(ABC): ) if self.use_sde and not isinstance(self.observation_space, gym.spaces.Box): - raise ValueError("generalized State-Dependent Exploration (gSDE) can only " "be used with continuous actions.") + raise ValueError("generalized State-Dependent Exploration (gSDE) can only be used with continuous actions.") def _wrap_env(self, env: GymEnv) -> VecEnv: if not isinstance(env, VecEnv): @@ -349,8 +349,11 @@ class BaseAlgorithm(ABC): # noinspection PyArgumentList model = cls( - policy=data["policy_class"], env=env, device="auto", _init_setup_model=False - ) # pytype: disable=not-instantiable,wrong-keyword-args + policy=data["policy_class"], + env=env, + device="auto", + _init_setup_model=False, # pytype: disable=not-instantiable,wrong-keyword-args + ) # load parameters model.__dict__.update(data) diff --git a/stable_baselines3/common/buffers.py b/stable_baselines3/common/buffers.py index c5e3a3f..e69da39 100644 --- a/stable_baselines3/common/buffers.py +++ b/stable_baselines3/common/buffers.py @@ -327,7 +327,7 @@ class RolloutBuffer(BaseBuffer): self.returns = self.advantages + self.values def add( - self, obs: np.ndarray, action: np.ndarray, reward: np.ndarray, done: np.ndarray, value: th.Tensor, log_prob: th.Tensor, + self, obs: np.ndarray, action: np.ndarray, reward: np.ndarray, done: np.ndarray, value: th.Tensor, log_prob: th.Tensor ) -> None: """ :param obs: (np.ndarray) Observation diff --git a/stable_baselines3/common/env_checker.py b/stable_baselines3/common/env_checker.py index a0659b9..558a9fe 100644 --- a/stable_baselines3/common/env_checker.py +++ b/stable_baselines3/common/env_checker.py @@ -82,19 +82,19 @@ def _check_obs(obs: Union[tuple, dict, np.ndarray, int], observation_space: spac if not isinstance(observation_space, spaces.Tuple): assert not isinstance( obs, tuple - ), "The observation returned by the `{}()` " "method should be a single value, not a tuple".format(method_name) + ), "The observation returned by the `{}()` method should be a single value, not a tuple".format(method_name) # The check for a GoalEnv is done by the base class if isinstance(observation_space, spaces.Discrete): assert isinstance(obs, int), "The observation returned by `{}()` method must be an int".format(method_name) elif _enforce_array_obs(observation_space): - assert isinstance(obs, np.ndarray), "The observation returned by `{}()` " "method must be a numpy array".format( + assert isinstance(obs, np.ndarray), "The observation returned by `{}()` method must be a numpy array".format( method_name ) assert observation_space.contains( obs - ), "The observation returned by the `{}()` " "method does not match the given observation space".format(method_name) + ), "The observation returned by the `{}()` method does not match the given observation space".format(method_name) def _check_returned_values(env: gym.Env, observation_space: spaces.Space, action_space: spaces.Space) -> None: @@ -187,9 +187,9 @@ def check_env(env: gym.Env, warn: bool = True, skip_render_check: bool = True) - :param skip_render_check: (bool) Whether to skip the checks for the render method. True by default (useful for the CI) """ - assert isinstance(env, gym.Env), ( - "You environment must inherit from gym.Env class " " cf https://github.com/openai/gym/blob/master/gym/core.py" - ) + assert isinstance( + env, gym.Env + ), "You environment must inherit from gym.Env class cf https://github.com/openai/gym/blob/master/gym/core.py" # ============= Check the spaces (observation and action) ================ _check_spaces(env) diff --git a/stable_baselines3/common/off_policy_algorithm.py b/stable_baselines3/common/off_policy_algorithm.py index 15bebd7..e31558e 100644 --- a/stable_baselines3/common/off_policy_algorithm.py +++ b/stable_baselines3/common/off_policy_algorithm.py @@ -163,8 +163,11 @@ class OffPolicyAlgorithm(BaseAlgorithm): **self.replay_buffer_kwargs ) self.policy = self.policy_class( - self.observation_space, self.action_space, self.lr_schedule, **self.policy_kwargs - ) # pytype:disable=not-instantiable + self.observation_space, + self.action_space, + self.lr_schedule, + **self.policy_kwargs # pytype:disable=not-instantiable + ) self.policy = self.policy.to(self.device) def save_replay_buffer(self, path: Union[str, pathlib.Path, io.BufferedIOBase]) -> None: @@ -328,16 +331,10 @@ class OffPolicyAlgorithm(BaseAlgorithm): fps = int(self.num_timesteps / (time.time() - self.start_time)) logger.record("time/episodes", self._episode_num, exclude="tensorboard") if len(self.ep_info_buffer) > 0 and len(self.ep_info_buffer[0]) > 0: - logger.record( - "rollout/ep_rew_mean", safe_mean([ep_info["r"] for ep_info in self.ep_info_buffer]), - ) - logger.record( - "rollout/ep_len_mean", safe_mean([ep_info["l"] for ep_info in self.ep_info_buffer]), - ) + logger.record("rollout/ep_rew_mean", safe_mean([ep_info["r"] for ep_info in self.ep_info_buffer])) + logger.record("rollout/ep_len_mean", safe_mean([ep_info["l"] for ep_info in self.ep_info_buffer])) logger.record("time/fps", fps) - logger.record( - "time/time_elapsed", int(time.time() - self.start_time), exclude="tensorboard", - ) + logger.record("time/time_elapsed", int(time.time() - self.start_time), exclude="tensorboard") logger.record("time/total timesteps", self.num_timesteps, exclude="tensorboard") if self.use_sde: logger.record("train/std", (self.actor.get_std()).mean().item()) diff --git a/stable_baselines3/common/on_policy_algorithm.py b/stable_baselines3/common/on_policy_algorithm.py index 8358569..f84d18f 100644 --- a/stable_baselines3/common/on_policy_algorithm.py +++ b/stable_baselines3/common/on_policy_algorithm.py @@ -117,8 +117,8 @@ class OnPolicyAlgorithm(BaseAlgorithm): self.lr_schedule, use_sde=self.use_sde, device=self.device, - **self.policy_kwargs - ) # pytype:disable=not-instantiable + **self.policy_kwargs # pytype:disable=not-instantiable + ) self.policy = self.policy.to(self.device) def collect_rollouts(