Avoid metaclass conflicts when inheriting from gym.Env

[YouJiacheng]

Description

Using __init_subclass__ instead of metaclass, suggested by PEP 487(introduced in python3.6).
As a result, downstream package can safely inherit from gym.Env and abc.ABC/Protocol/...(classes with metaclass != type), or set their own metaclass without making a intermediate metaclass inherited from custom metaclass and type(gym.Env).

For example, https://github.com/rlworkgroup/metaworld use abc.ABC/abc.ABCMeta, thus it was broken by the metaclass introduced in gym-0.25.0.
https://github.com/rlworkgroup/metaworld/blob/18118a28c06893da0f363786696cc792457b062b/metaworld/envs/mujoco/mujoco_env.py#L31
https://github.com/rlworkgroup/metaworld/blob/18118a28c06893da0f363786696cc792457b062b/metaworld/envs/mujoco/sawyer_xyz/sawyer_xyz_env.py#L14

import abc, gym
class SomeAbstractEnvUseABCMeta(gym.Env, metaclass=abc.ABCMeta): pass
class SomeAbstractEnvUseABC(gym.Env, abc.ABC): pass

Above code works on gym<0.25.0, but crashs on gym==0.25.0

Type of change

• Bug fix (non-breaking change which fixes an issue)

If user-code/downstream library doesn't depend on type(gym.Env), it should not be a breaking change, but there is a rare case that workaround for metaclass introduced in gym-0.25.0 is used:

import abc, gym
class AbstractEnvMeta(type(gym.Env), abc.ABCMeta): pass
class SomeAbstractEnv(gym.Env, metaclass=AbstractEnvMeta): pass

Above code will crash if issubclass(abc.ABCMeta, type(gym.Env)), which is true without decorator metaclass introduced in gym-0.25.0 since type(gym.Env) is type, because of MRO conflict. i.e., above code will crash on gym<0.25.0 and after this change.
I want to note that following code won't crash on any version of gym or after this change:

import abc, gym
class AbstractEnvMeta(abc.ABCMeta, type(gym.Env)): pass
class SomeAbstractEnv(gym.Env, metaclass=AbstractEnvMeta): pass

Checklist:

• I have run the pre-commit checks with pre-commit run --all-files (see CONTRIBUTING.md instructions to set it up)
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation
• 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

[younik]

Hello, thanks for addressing it!
LGTM

(You will probably just need to fix pre-commit hook)

[RedTachyon]

Any reason why this can't be cls.render = _deprecate_mode(cls.render)? If not, I'd prefer that for simplicity

[YouJiacheng]

Given "render" in vars(cls) is True, vars(cls)["render"] is the same as cls.render in 99.9% cases, except __getattr__ being overridden.
I want to maximize the consistency between exist-check and access. Ideally I want to write if "render" in vars(cls): vars(cls).update(render=vars(cls)["render"]), but vars(cls) is read-only.
FWIW, the reason why try...except or hasattr check + cls.render cannot be used is that cls.render can come from base classes.
BTW, do you think following code is of simplicity?

render_func = vars(cls).get("render")
if render_func is not None:
    cls.render = render_func

[RedTachyon]

I like the alternative you presented here, of course with the added wrapper

[RedTachyon]

Left one comment, looks good otherwise