Skip to content

Move mlagents.envs #2902

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 3 commits into from
Closed

Move mlagents.envs #2902

wants to merge 3 commits into from

Conversation

chriselion
Copy link
Contributor

Rename mlagents.envs to mlagentsenvs.envs.

The old naming scheme relied on PEP420 which was causing problems with pylint (had to disable a warning) and mypy (couldn't enable --namespace-packages without hitting python/mypy#7510, but this missed some errors).

Open to suggestions on the name. We could remove the envs directory so the imports are e.g. mlagentsenvs.environment instead of mlagents.envs.environment

Currently this just moves the files and updates the imports and the proto files. I'll do another pass to fix any doc changes if we're OK with the move and the name. Expecting mypy to fail until the newly uncovered errors are fixed.

@chriselion chriselion changed the title Develop move mlagents.envs Move mlagents.envs Nov 13, 2019
@harperj
Copy link
Contributor

harperj commented Nov 13, 2019

What about mlagents_envs.environment?

I'm also beginning to think the right thing may be to just re-combine the two packages. Have you thought about that option?

@surfnerd
Copy link
Contributor

I think removing the envs directory makes sense here. If there were any sibling directories, or code maybe it would make less sense. Alas, there is nothing under the mlagents folder other than envs.

@surfnerd
Copy link
Contributor

does the windows protobuf generation script also need to be updated?

@chriselion
Copy link
Contributor Author

Cool, will probably redo this PR instead of fixing the merge conflicts.

I'll make sure to update the windows script, although it's probably less important now that we removed the Custom* protos.

@chriselion
Copy link
Contributor Author

Closing this for now, will revisit soon

@chriselion chriselion closed this Dec 5, 2019
@chriselion chriselion deleted the develop-move-mlagents.envs branch January 23, 2020 23:39
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 16, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants