Fixes to the new registration

[RedTachyon]

Update to #2748, turns out a few comments from @Markus28 didn't show up, and some of them actually pointed out bugs, so it's necessary to get them fixed.

[pseudo-rnd-thoughts]

For the bugs that were found, is it possible to add tests for them to prevent such bugs from happening again?

[Markus28]

Could you also explain how this function

gym/gym/envs/registration.py

Line 223 in dcae553

def load_env_plugins(entry_point: str = "gym.envs") -> None:

is supposed to be used?

[Markus28]

Is gym.envs.registry supposed to remain backwards compatible? The scripts for gym-docs, for instance, are currently breaking on the master branch because they use gym.envs.registry.all().

[RedTachyon]

Could you also explain how this function

gym/gym/envs/registration.py

Line 223 in dcae553

def load_env_plugins(entry_point: str = "gym.envs") -> None:

is supposed to be used?

That's out of scope for this, it was introduced by @JesseFarebro a while ago, and by now I think I mostly understand what it does, and it does desperately need documentation (which I'll probably have to discuss with Jesse), but it's mostly orthogonal from these updates.

[RedTachyon]

Is gym.envs.registry supposed to remain backwards compatible?

My approach here was to keep it "mostly compatible for reasonable uses", which is as vague as it sounds.

registry.all() is imo something that shouldn't have really happened in the first place, and supporting it for backwards compatiblity would be at least somewhat cursed (although it would be possible, and I have an idea on how to keep the cursedness to a minimum)

That being said, it is the exact equivalent of registry.values() in the new code. Old registry was a wrapper around a pimped-up dictionary. New registry is just a dictionary, and I like it being just a dictionary (simple is better than complex)

[RedTachyon]

For the bugs that were found, is it possible to add tests for them to prevent such bugs from happening again?

I'll try to add some tests soon

[RedTachyon]

I added a test, ultimately the main issue was around handling the namespace context manager, which is in practice only used in ALE so far, and tests don't cover that. So the test does a basic sanity check that the namespace gets added the way we want it to be.

[pseudo-rnd-thoughts]

LGTM @jkterry1