Add shimmy for atari and removes the gym compatibility for the shimmy versions

[pseudo-rnd-thoughts]

Shimmy is a farama project that provides compatibility for non-gymnasium or pettingzoo reinforcement learning environments.

As Ale-Py has not updated to gymnasium yet, we add the compatibility wrappers to shimmy in the meantime.
We have moved the gym compatibility wrappers to shimmy as well therefore, have removed the environments from gymnasium

[RedTachyon]

A few comments, and two more things to consider:

1. Can we have some sort of a warning or a deprecation for the current gym.make("GymCompatibility...")? If someone calls it without installing shimmy, it will inexplicably fail. We can either keep the current class but add a deprecation warning, or maybe make it so that there's a descriptive warning when someone tries to do it without shimmy.
2. Can we add some documentation on how to correctly create gym envs as gymnasium envs? Will it be something like gym.make("shimmy:GymConversion", env_id=...) or maybe gym.make("Shimmy/GymConversion", env_id=...)? Both for concrete environments we might want to have registered (like atari), and for arbitrary other environments

[RedTachyon]

This used to happen before everything, now it happens after. It's a change that I'm not sure about either way.

If we assume that people know what they're doing (which in general is a good assumption to keep for gymnasium), then it's not a problem - if you have a package that overwrites CartPole-v1 for whatever reason, then you probably want to use that instead of the default gym envs.

On the other hand, this specific situation can be a bit confusing due to how the plugin system works. It doesn't really need any active decisions from the user - just having the library installed calls the plugins if I recall correctly. So if some third-party library is installed and replaces the built-in envs, the built-ins become inaccessible, which might lead to confusing bugs and be all sorts of problematic.

I'm slightly leaning to moving this before the built-in registrations, what do you think?

[pseudo-rnd-thoughts]

Originally, the reason to move it was for none of these reasons. I did it for the pprint_registry function as a dict remembers the order of insert, previously the function would have "external" environment first then "internal" environment. By moving the function to the end, this means that the "internal" environments are shown first.

To answer your questions

1. Given the previous implementation, if users "overwrote" the cartpole-v1 for example, this was not possible as my understanding for the order of operations was 1. load external environment then 2. load internal environment (overriding external environment). Second, I would have thoughts, users just make a cartpole-v2 if they wanted do this

I am very happy to move this back to the original location and don't feel strongly about the decision

[RedTachyon]

Second, I would have thoughts, users just make a cartpole-v2 if they wanted do this

Yes, that would definitely be good practice.

In principle the whole thing I brough up here shouldn't happen ever, I'm just thinking of edge cases and worst-case scenarios.

Let's say someone makes a library called RLify meant to do something related to RL. Among its dependencies there's totally-innocent-package so that gets automatically installed. It so happens that it works its way into the plugin system and registers an environment as CartPole-v1 with the code as follows:

class CartPole(gym.Env):
...
    def step(self, action):
        mine_crypto()
        ...

Now, of course we can't prevent things like that happening in third-party code. But what worries me is how indetectible this would be - it happens if you have that library installed and use Gymnasium. You don't need to import that library, you don't need to be aware of it.

This is obviously over-exaggerated. A more realistic situation might be that RLify registers their own CartPole-v1 with the good intentions of improving its performance or something like that. This quietly replaces built-in cartpole. A few releases later it breaks, and then we'll have issues with perfectly reasonable code which only uses gymnasium, which completely breaks stuff.

Which also means that if everything is identical (system, python version, gymnasium version) except for one installed library, we can't be sure that import gymnasium as gym; gym.make("CartPole-v0") actually creates the same object.

This is an argument against the plugin system as a whole and I've had problems with it ever since I discovered it, but didn't have the time/energy to actually open up a larger discussion. It saves one line of code per library, but makes things a whole bunch more confusing.

[pseudo-rnd-thoughts]

What would you propose doing in the meantime?

[RedTachyon]

I suppose we can keep it as is for now, since the plugin feature made its way into the repo, we'll just need to think if we want to do something about it before 1.0 at some point in the future

[RedTachyon]

Do we maybe want to move these tests into shimmy instead? My rationale is that right now, gymnasium doesn't really interact with gym directly in any way, so it shouldn't even really be a testing dependency. The interaction goes via shimmy, so shimmy should take care of it. Similarly to how we don't have any tests for dm-env here

[pseudo-rnd-thoughts]

True, Im happy to remove the test except for testing that the environment exist in the registry as expected

[RedTachyon]

How confident are we that e.g. shimmy==0.2 will not break anything when it happens?

[pseudo-rnd-thoughts]

I should have added a note here, my intentions is that shimmy 1.0 will be for when ale-py is updated to gymnasium. Therefore, if users installed gymnasium==0.27 with shimmy>=1.0 then atari won't be installed.

But your point about 0.2.0 not breaking things is fair, I think that is unlikely and we can test for it however I can remove the upper limit if you think this might happen

[RedTachyon]

This is a quiet change, why?

[pseudo-rnd-thoughts]

I can remove, I think I put this in the wrong branch. It will be necessary for python 3.11

[pseudo-rnd-thoughts]

1. As shimmy is now a necessary install within setup.py then this case shouldn't exist. I am happy to look at using a different env id, i.e. shimmy/GymV26Compatibility-v0 etc and adding a deprecation warning. However, I don't think this is necessary. We could do gym/V26Compatibility-v0
2. As shimmy uses the entry points, whenever someone installs gymnasium then the compatibility should be accessible and part of the registry. The only issue is for devs who need to pip install -e . to access the environments automatically.

[RedTachyon]

LGTM