[Bug fix] remove mujoco-py import error for v4+ MuJoCo environments

[MischaPanch]

Description

Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.

Fixes #896

Type of change

Please delete options that are not relevant.

• Documentation only change (no code changed)
• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• This change requires a documentation update

Screenshots

Please attach before and after screenshots of the change if applicable.

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

[MischaPanch]

As discussed in the issue, I split up the mujoco and mujoco-py envs and adjusted imports. This solves the issue (I tested locally)

I used the git split utility, so the "new" files contain the commit history. For example, the new base_env file:

[Kallinteris-Andreas]

As I said in the issue with regard to baseMujocoEnv, do not move it from mujoco.py instead create copy in mujoco_py_env.py

[MischaPanch]

As I said in the issue with regard to baseMujocoEnv, do not move it from mujoco.py instead create copy in mujoco_py_env.py

See my answer to your comments about the imports in init. Three files are needed to enable importing from say mujoco_py without importing from mujoco

[MischaPanch]

Your pre-commit pipeline was not running on my machine because of some pkg_resources related error. Unfortunately, there was no setup instructions in the contributing.md. Could you either reformat the code yourself or share the development setup with me?

[Kallinteris-Andreas]

Sorry for the miscommunication on my part, (since my initial comment in your issue and now, we have marked mujoco-py environments as explicitly deprecated)

Here is are the updated requirements happen:
files:

• mujoco_env.py: keep baseMujocoEnv & MujocoEnv
• mujoco_py_env.py: place a copy of baseMujocoEnv (and rename it to baseMujocoPyEnv) & MujocoPyEnv

motivation: MujocoPyEnv should be in its own file with nothing external affecting it, BaseMujocoEnv will eventually be "merged" with MujocoEnv

[MischaPanch]

I see, makes sense. I'll adjust accordingly and write a block comment in the reasons for the code duplication

[Kallinteris-Andreas]

Hey, sorry for being late review, I missed the request for review

mostly looks good

[MischaPanch]

Hey, sorry for being late review, I missed the request for review

@Kallinteris-Andreas no problem, happens to me too :)

Done now. One more import in a test file was adjusted to be consistent with the remaining imports (I did a search-replace)

[Kallinteris-Andreas]

This is looking almost done
To fix the failing test
change

assert isinstance(env, BaseMujocoEnv)

to

assert isinstance(env, BaseMujocoEnv) or isinstance(env, BaseMujocoPyEnv)

then rebase to main, and we should be ready for merge

[MischaPanch]

@Kallinteris-Andreas

1. I adjusted the isinstance (using tuple instead of multiple checks)
2. Adjusted imports as you asked
3. Rebasing resulted in conflicts, so I merged main instead

[Kallinteris-Andreas]

Excellent, Thank you