feat: throw error on invalid render mode 🧑‍💻

[robertoschiavone]

Description

I changed the UserWarning thrown by VideoRecorder into a ValueError that gets thrown if render_mode is an invalid value. The exception is mirrored inside the constructor of RecordVideo so that it fails as soon as possible.

Motivation

Right now, when you wrap an env in a RecordVideo, the code throws only an user warning at the end of the training when it tries to save a video but render_mode is set to None, or in general to any other value except rgb_array and rgb_array_list.

It would be nice to check the render mode early, ideally inside the RecordVideo constructor, so that the training doesn't have to be restarted in case the parameter was accidentally left unspecified.

Before	After
env = gym.make(env_name) env = RecordVideo(env, directory) train(model, env) ... # throws user warning here, without recording # you now have to start from scratch	env = gym.make(env_name) env = RecordVideo(env, directory) # will throw exception 🚫
	env = gym.make(env_name, render_mode="rgb_array") env = RecordVideo(env, directory) # will continue execution ✅

Fixes #560

Type of change

• New feature (non-breaking change which adds functionality)

Checklist:

• I have run the pre-commit checks with pre-commit run --all-files (see CONTRIBUTING.md instructions to set it up)
• My changes generate no new warnings
• I have added tests that prove my fix is effective or that my feature works
• New and existing unit tests pass locally with my changes

[robertoschiavone]

I focused on keeping all tests in test_record_video.py green, but I didn’t add new ones. Let me know if I should cover my changes with extra tests.

[pseudo-rnd-thoughts]

While I agree with the idea that raising an error is more helpful when a warning a large number of times.
I'm uncertain if we should be limiting the possible render modes that can be used with RecordVideo

For example, MuJoCo has a depth_array render mode that is uncommon but to my knowledge should be compatible with RecordVideo.

I see three possible solutions

1. Keep the code as it is
2. Raise an error only when the render_mode is None, as this is the most common case when an issue will occur
3. Add a parameter confirm_uncommon_render_mode that allows uncommon render modes to be used but unless oped-in then an error will occur

Personally, I think the second option is the best one

[robertoschiavone]

I would go with 2 but if you agree I would also add the check for the other known and documented render modes, like this:

if not render_mode or render_mode is in ["human", "ansi", "ansi_list"]:
    raise Exception

since we know for sure they cannot be rendered.

Anything else (["rgb_array", "rgb_array_list", whatever MuJoCo does]) is fair game.

[pseudo-rnd-thoughts]

Looking at the new experiment RecordVideoV0, I would follow what it does - https://github.com/Farama-Foundation/Gymnasium/blob/c6bf140018593331b8fe733db528d0e40ab8ef7a/gymnasium/experimental/wrappers/rendering.py#L144C9-L144C9

[robertoschiavone]

I tried to make it as close as possible to the experimental RecordVideoV0.

[pseudo-rnd-thoughts]

Thanks for making the change