Add optional install tests (#14)

Author: pseudo-rnd-thoughts

.github/workflows/build-test.yml

@@ -13,7 +13,7 @@ jobs:
     steps:
       - uses: actions/checkout@v2
       - run: |
-           docker build -f py.Dockerfile \
+           docker build -f bin/all.Dockerfile \
              --build-arg PYTHON_VERSION=${{ matrix.python-version }} \
              --tag shimmy-docker .
       - name: Run tests

.github/workflows/optional-install-test.yml

@@ -0,0 +1,37 @@
+name: build
+on: [pull_request, push]
+
+permissions:
+  contents: read # to fetch code (actions/checkout)
+
+jobs:
+  optional-install:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v2
+
+        # Atari
+      - run: |
+           docker build -f bin/atari.Dockerfile \
+             --build-arg PYTHON_VERSION='3.10' \
+             --tag shimmy-atari-docker .
+      - name: Run atari tests
+        run: docker run shimmy-atari-docker pytest tests/test_atari.py
+
+        # dm-control
+      - run: |
+          docker build -f bin/dm_control.Dockerfile \
+            --build-arg PYTHON_VERSION='3.10' \
+            --tag shimmy-dm-control-docker .
+      - name: Run dm-control tests
+        run: docker run shimmy-dm-control-docker pytest tests/test_dm_control.py
+
+        # openspiel
+      - run: |
+          docker build -f bin/openspiel.Dockerfile \
+            --build-arg PYTHON_VERSION='3.10' \
+            --tag shimmy-openspiel-docker .
+      - name: Run openspiel tests
+        run: docker run shimmy-openspiel-docker pytest tests/test_openspiel.py
+
+        # add more options

py.Dockerfile bin/all.Dockerfilepy.Dockerfile renamed to bin/all.Dockerfile

@@ -20,6 +20,6 @@ RUN apt-get -y update \
 COPY . /usr/local/shimmy/
 WORKDIR /usr/local/shimmy/
 
-RUN pip install .[testing] --no-cache-dir
+RUN pip install ".[all, testing]" --no-cache-dir
 
 ENTRYPOINT ["/usr/local/shimmy/bin/docker_entrypoint"]

bin/atari.Dockerfile

@@ -0,0 +1,25 @@
+# A Dockerfile that sets up a full shimmy install with test dependencies
+ARG PYTHON_VERSION
+FROM python:$PYTHON_VERSION
+
+SHELL ["/bin/bash", "-o", "pipefail", "-c"]
+
+RUN apt-get -y update \
+    && apt-get install --no-install-recommends -y \
+    unzip \
+    libglu1-mesa-dev \
+    libgl1-mesa-dev \
+    libosmesa6-dev \
+    xvfb \
+    patchelf \
+    ffmpeg cmake \
+    && apt-get autoremove -y \
+    && apt-get clean \
+    && rm -rf /var/lib/apt/lists/*
+
+COPY . /usr/local/shimmy/
+WORKDIR /usr/local/shimmy/
+
+RUN pip install ".[atari, testing]" --no-cache-dir
+
+ENTRYPOINT ["/usr/local/shimmy/bin/docker_entrypoint"]

bin/dm_control.Dockerfile

@@ -0,0 +1,25 @@
+# A Dockerfile that sets up a full shimmy install with test dependencies
+ARG PYTHON_VERSION
+FROM python:$PYTHON_VERSION
+
+SHELL ["/bin/bash", "-o", "pipefail", "-c"]
+
+RUN apt-get -y update \
+    && apt-get install --no-install-recommends -y \
+    unzip \
+    libglu1-mesa-dev \
+    libgl1-mesa-dev \
+    libosmesa6-dev \
+    xvfb \
+    patchelf \
+    ffmpeg cmake \
+    && apt-get autoremove -y \
+    && apt-get clean \
+    && rm -rf /var/lib/apt/lists/*
+
+COPY . /usr/local/shimmy/
+WORKDIR /usr/local/shimmy/
+
+RUN pip install ".[dm-control, testing]" --no-cache-dir
+
+ENTRYPOINT ["/usr/local/shimmy/bin/docker_entrypoint"]

bin/openspiel.Dockerfile

@@ -0,0 +1,25 @@
+# A Dockerfile that sets up a full shimmy install with test dependencies
+ARG PYTHON_VERSION
+FROM python:$PYTHON_VERSION
+
+SHELL ["/bin/bash", "-o", "pipefail", "-c"]
+
+RUN apt-get -y update \
+    && apt-get install --no-install-recommends -y \
+    unzip \
+    libglu1-mesa-dev \
+    libgl1-mesa-dev \
+    libosmesa6-dev \
+    xvfb \
+    patchelf \
+    ffmpeg cmake \
+    && apt-get autoremove -y \
+    && apt-get clean \
+    && rm -rf /var/lib/apt/lists/*
+
+COPY . /usr/local/shimmy/
+WORKDIR /usr/local/shimmy/
+
+RUN pip install ".[openspiel, testing]" --no-cache-dir
+
+ENTRYPOINT ["/usr/local/shimmy/bin/docker_entrypoint"]

setup.py

@@ -33,13 +33,14 @@ def get_version():
 header_count, long_description = get_description()
 
 extras = {
+    "gym": ["gym>=0.26"],
+    "atari": ["ale-py~=0.8.0"],
     # "imageio" should be "gymnasium[mujoco]>=0.26" but there are install conflicts
     "dm-control": ["dm-control>=1.0.8", "imageio"],
     "openspiel": ["open_spiel>=1.2", "pettingzoo>=1.22"],
-    "atari": ["ale-py~=0.8.0"],
 }
 extras["all"] = list({lib for libs in extras.values() for lib in libs})
-extras["testing"] = extras["all"] + [
+extras["testing"] = [
     "pytest==7.1.3",
     "pillow>=9.3.0",
     "autorom[accept-rom-license]~=0.4.2",

shimmy/openai_gym_compatibility.py

@@ -1,4 +1,5 @@
 """Compatibility wrappers for OpenAI gym V22 and V26."""
+# pyright: reportGeneralTypeIssues=false, reportPrivateImportUsage=false
 from __future__ import annotations
 
 import sys
@@ -80,9 +81,7 @@ def __init__(
         self.metadata = getattr(self.gym_env, "metadata", {"render_modes": []})
         self.render_mode = self.gym_env.render_mode
         self.reward_range = getattr(self.gym_env, "reward_range", None)
-        self.spec = getattr(  # pyright: ignore[reportGeneralTypeIssues]
-            self.gym_env, "spec", None
-        )
+        self.spec = getattr(self.gym_env, "spec", None)
 
     def reset(
         self, seed: int | None = None, options: dict | None = None
@@ -183,24 +182,24 @@ def __init__(
             make_kwargs = {}
 
         if env is not None:
-            self.gym_env = env
+            gym_env = env
         elif env_id is not None:
-            self.gym_env = gym.make(env_id, **make_kwargs)
+            gym_env = gym.make(env_id, **make_kwargs)
         else:
             raise MissingArgument(
                 "Either env_id or env must be provided to create a legacy gym environment."
             )
-        self.observation_space = _convert_space(self.gym_env.observation_space)
-        self.action_space = _convert_space(self.gym_env.action_space)
+        self.observation_space = _convert_space(gym_env.observation_space)
+        self.action_space = _convert_space(gym_env.action_space)
 
-        self.gym_env = _strip_default_wrappers(self.gym_env)
+        gym_env = _strip_default_wrappers(gym_env)
 
-        self.metadata = getattr(self.gym_env, "metadata", {"render_modes": []})
+        self.metadata = getattr(gym_env, "metadata", {"render_modes": []})
         self.render_mode = render_mode
-        self.reward_range = getattr(self.gym_env, "reward_range", None)
-        self.spec = getattr(  # pyright: ignore[reportGeneralTypeIssues]
-            self.gym_env, "spec", None
-        )
+        self.reward_range = getattr(gym_env, "reward_range", None)
+        self.spec = getattr(gym_env, "spec", None)
+
+        self.gym_env: LegacyV22Env = gym_env
 
     def reset(
         self, seed: int | None = None, options: dict | None = None

shimmy/registration.py

@@ -6,10 +6,8 @@
 from typing import Any, Callable, Mapping, NamedTuple, Sequence
 
 import numpy as np
-from ale_py.roms import utils as rom_utils
 from gymnasium.envs.registration import register
 
-from shimmy.dm_control_compatibility import DmControlCompatibility
 from shimmy.utils.envs_configs import (
     ALL_ATARI_GAMES,
     DM_CONTROL_MANIPULATION_ENVS,
@@ -25,6 +23,8 @@ def _register_dm_control_envs():
     except ImportError:
         return
 
+    from shimmy.dm_control_compatibility import DmControlCompatibility
+
     # Add generic environment support
     def _make_dm_control_generic_env(env, **render_kwargs):
         return DmControlCompatibility(env, **render_kwargs)
@@ -128,6 +128,8 @@ def _register_atari_configs(
     configs: Sequence[GymConfig],
     prefix: str = "",
 ):
+    from ale_py.roms import utils as rom_utils
+
     for rom in roms:
         for obs_type in obs_types:
             for config in configs:
@@ -162,6 +164,11 @@ def _register_atari_configs(
 
 
 def _register_atari_envs():
+    try:
+        import ale_py
+    except ImportError:
+        return
+
     frameskip: dict[str, int] = defaultdict(lambda: 4, [("space_invaders", 3)])
 
     configs = [

tests/test_gym.py

@@ -0,0 +1,67 @@
+"""Tests that gym compatibility environment work as expected."""
+
+import warnings
+
+import gym
+import gymnasium
+import pytest
+from gymnasium.error import Error
+from gymnasium.utils.env_checker import check_env
+
+CHECK_ENV_IGNORE_WARNINGS = [
+    f"\x1b[33mWARN: {message}\x1b[0m"
+    for message in [
+        "This version of the mujoco environments depends on the mujoco-py bindings, which are no longer maintained and may stop working. Please upgrade to the v4 versions of the environments (which depend on the mujoco python bindings instead), unless you are trying to precisely replicate previous works).",
+        "A Box observation space minimum value is -infinity. This is probably too low.",
+        "A Box observation space maximum value is -infinity. This is probably too high.",
+        "For Box action spaces, we recommend using a symmetric and normalized space (range=[-1, 1] or [0, 1]). See https://stable-baselines3.readthedocs.io/en/master/guide/rl_tips.html for more information.",
+    ]
+]
+
+# We do not test Atari environment's here because we check all variants of Pong in test_envs.py (There are too many Atari environments)
+CLASSIC_CONTROL_ENVS = [
+    env_id
+    for env_id, spec in gym.envs.registry.items()  # pyright: ignore[reportGeneralTypeIssues]
+    if ("classic_control" in spec.entry_point)
+]
+
+
+@pytest.mark.parametrize(
+    "env_id", CLASSIC_CONTROL_ENVS, ids=[env_id for env_id in CLASSIC_CONTROL_ENVS]
+)
+def test_gym_conversion_by_id(env_id):
+    """Tests that the gym conversion works through specifying the env_id."""
+    env = gymnasium.make("GymV26Environment-v0", env_id=env_id).unwrapped
+
+    with warnings.catch_warnings(record=True) as caught_warnings:
+        check_env(env)
+
+    for warning in caught_warnings:
+        if (
+            isinstance(warning.message, Warning)
+            and warning.message.args[0] not in CHECK_ENV_IGNORE_WARNINGS
+        ):
+            raise Error(f"Unexpected warning: {warning.message}")
+
+    env.close()
+
+
+@pytest.mark.parametrize(
+    "env_id", CLASSIC_CONTROL_ENVS, ids=[env_id for env_id in CLASSIC_CONTROL_ENVS]
+)
+def test_gym_conversion_instantiated(env_id):
+    """Tests that the gym conversion works with an instantiated gym environment."""
+    env = gym.make(env_id)
+    env = gymnasium.make("GymV26Environment-v0", env=env).unwrapped
+
+    with warnings.catch_warnings(record=True) as caught_warnings:
+        check_env(env)
+
+    for warning in caught_warnings:
+        if (
+            isinstance(warning.message, Warning)
+            and warning.message.args[0] not in CHECK_ENV_IGNORE_WARNINGS
+        ):
+            raise Error(f"Unexpected warning: {warning.message}")
+
+    env.close()