Move atari/mujoco helpers into package code - #1293
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bce09ae72e
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| ScaledObsInputActionReprNet, | ||
| layer_init, | ||
| ) | ||
| from tianshou.env.atari.atari_wrapper import ( |
There was a problem hiding this comment.
Importing tianshou.env.atari now eagerly imports atari_wrapper, which unconditionally imports cv2; this makes from tianshou.env.atari import C51Net fail in environments that install vizdoom/network dependencies but not the optional Atari/OpenCV stack. Before this commit, users could import C51Net/DQNet directly from atari_network without requiring cv2, so the new __init__ introduces a regression that can break the updated vizdoom examples at import time.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch. Fixed in 8f1992b: atari_wrapper imports (which pull in cv2) are now deferred via __getattr__. Network classes (C51Net, DQNet, etc.) remain eagerly importable since they don't depend on cv2. This way, vizdoom examples that only need the network utilities won't break if the Atari/OpenCV stack isn't installed.
Move mujoco_env.py from examples/ into tianshou/env/mujoco/ and add __init__.py for both tianshou/env/atari/ and tianshou/env/mujoco/ to enable clean imports like `from tianshou.env.mujoco import make_mujoco_env`. The original examples/mujoco/mujoco_env.py is preserved as a backward- compatible re-export shim. Closes thu-ml#988 Signed-off-by: Lidang-Jiang <lidangjiang@gmail.com>
With the import change from local mujoco_env to tianshou.env.mujoco, mypy now fully resolves the return type of make_mujoco_env and flags Space[Any] attribute accesses (.low, .high, .n). Add isinstance assertions for Box spaces and remove dead .n fallback branches. Signed-off-by: Lidang-Jiang <lidangjiang@gmail.com>
Signed-off-by: Lidang-Jiang <lidangjiang@gmail.com>
atari_wrapper.py imports cv2 at the top level. Eagerly importing it from __init__.py breaks environments that only need the network utilities (C51Net, DQNet) without the Atari/OpenCV stack. Use __getattr__ to defer the atari_wrapper import until a wrapper symbol is actually accessed. Signed-off-by: Lidang-Jiang <lidangjiang@gmail.com>
8f1992b to
042e7be
Compare
1 participant
Summary
Addresses #988 — moves the mujoco helper code from
examples/into the tianshou package and adds proper__init__.pyfiles for both atari and mujoco submodules.Changes
New files (3):
tianshou/env/atari/__init__.py— re-exports all public symbols fromatari_wrapper.pyandatari_network.pytianshou/env/mujoco/__init__.py— exportsmake_mujoco_env,MujocoEnvFactory,MujocoEnvObsRmsPersistencetianshou/env/mujoco/mujoco_env.py— moved fromexamples/mujoco/mujoco_env.pyModified files (37):
from mujoco_env import ...→from tianshou.env.mujoco import ...from tianshou.env.atari.atari_wrapper import ...→from tianshou.env.atari import ...examples/mujoco/mujoco_env.py: converted to backward-compatible re-export shimNot modified:
tianshou/env/__init__.py— atari/mujoco have optional deps (cv2, mujoco), not auto-importedBefore (master branch)
After (this branch)
Test plan
pytest test/base/ -x— 149 passed, 3 skipped (identical to master)ruff checkandruff format --checkpassCloses #988