facebookresearch / torchbeast

A PyTorch Platform for Distributed RL
Apache License 2.0
734 stars 113 forks source link

Fixed unnecessary if statement #2

Closed jmribeiro closed 4 years ago

jmribeiro commented 4 years ago

Looking through your wrappers I found an unnecessary duplication of a condition.

facebook-github-bot commented 4 years ago

Hi jmribeiro! Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file.In order for us to review and merge your code, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks!

facebook-github-bot commented 4 years ago

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

heiner commented 4 years ago

Hey João!

Thanks for your PR and of course you are right that that's an unnecessary condition. In fact, there are many things wrong and/or slightly weird about this file.

However, we are not going to merge this PR as we want to keep this file as close as possible to its upstream version -- as it says in the header, this file was written by OpenAI, here: https://github.com/openai/baselines/blob/master/baselines/common/atari_wrappers.py If you are looking to clean up things in there (it could also use some linting), I'd suggest changing this upstream version.