Print the tuple-observation rejection as one sentence - #1306
DenisDrobyshev wants to merge 1 commit into
Conversation
Both raises split the message across two positional arguments, so
str(exception) rendered the pair rather than the sentence:
TypeError: ('Tuple observation space is not supported. ', 'Please change
it to array or dict space')
The trailing space on the first half shows the two were meant to be joined by
implicit concatenation, and the comma between them defeated it.
![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: 0ba90186c9
ℹ️ 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".
| """ | ||
| env: Any = DummyVectorEnv([_TupleObsEnv]) | ||
| if wrap_norm_obs: | ||
| env = VectorEnvNormObs(env) |
There was a problem hiding this comment.
When wrap_norm_obs is true, VectorEnvNormObs.reset() first calls the wrapped DummyVectorEnv.reset(), which raises from BaseVectorEnv.reset() before returning the tuple observation. Consequently, both parameter values exercise only the venvs.py change, and a regression in the newly patched VectorEnvNormObs exception would still pass this test. Use a stub vector environment whose reset() returns a tuple observation so the wrapper's own check is reached.
Useful? React with 👍 / 👎.
2 participants
Description
Both places that reject a tuple observation split the message across two
positional arguments, so
str(exception)renders the pair rather than thesentence that was written:
The trailing space on the first half shows the two were meant to be joined by
implicit concatenation; the comma between them defeated it. After:
The same two lines appear in
tianshou/env/venvs.py(BaseVectorEnv.reset)and
tianshou/env/venv_wrappers.py(VectorEnvNormObs.reset), so both arefixed and both are covered by the test.
An AST pass over
tianshou/finds no other built-in exception given more thanone positional argument.
Tests
test_tuple_obs_error_is_one_sentenceintest/base/test_env.py, parametrisedover the plain vector env and the
VectorEnvNormObswrapper so each patchedfile is exercised.
len(exception.args) == 1is the assertion that fails onmaster; both cases pass here.test/base/test_env.py -k "tuple_obs or norm_obs or env_obs"is 4 passed.ruff formatis clean on the three files.ruff checkreports the same singlepre-existing
PLW0108atvenvs.py:473before and after this change, which isuntouched here.