[RLlib] Replace ordinary pygame imports by try_imports #31332
Conversation
Signed-off-by: Artur Niederfahrenhorst <[email protected]>
| from typing import Optional | ||
|
|
||
| import numpy as np |
There was a problem hiding this comment.
Just curious, is this a new sorting convention? Three blocks: a) included python modules, b) other third-party imports, c) ray imports?
There was a problem hiding this comment.
The three categories you describe (and the formating changes here) conform with PEP-8 I belive.
-> https://peps.python.org/pep-0008/#imports
I think we should adhere at least to PEP-8. The google style guide is optional.
I use PyCharm's "optimize imports" feature to do these, but PyCharm also groups "import x as y" vs "import x" and separates them which makes for apparent missing alphabetical order.
rllib/env/utils.py
Outdated
| error: Whether to raise an error if pyspiel cannot be imported. | ||
|
|
||
| Returns: | ||
| The tfp module. |
rllib/env/utils.py
Outdated
| The tfp module. | ||
|
|
||
| Raises: | ||
| ImportError: If error=True and tfp is not installed. |
There was a problem hiding this comment.
-> "pyspiel" instead of tfp
| import re | ||
|
|
||
| import numpy as np | ||
| from open_spiel.python.rl_environment import Environment |
There was a problem hiding this comment.
Dumb question: what was the difference between open_spiel and pyspiel again? Would we need to return the open_spiel package as well from try_import_pyspiel()?
There was a problem hiding this comment.
open spiel uses pygame, which is a game engine with a python interface.
There was a problem hiding this comment.
They are separate packages though.
We list pygame in our test requirements and open_spiel nowhere.
There was a problem hiding this comment.
I have added open_spiel to this PR with the same behaviour!
| import sys | ||
|
|
||
| import numpy as np | ||
| from open_spiel.python.rl_environment import Environment |
Signed-off-by: Artur Niederfahrenhorst <[email protected]>
Signed-off-by: Artur Niederfahrenhorst <[email protected]>
Signed-off-by: Artur Niederfahrenhorst <[email protected]>
Signed-off-by: Artur Niederfahrenhorst <[email protected]>
Signed-off-by: Artur Niederfahrenhorst <[email protected]>
Signed-off-by: Artur Niederfahrenhorst <[email protected]>
…oject#31332) Signed-off-by: Andrea Pisoni <[email protected]>
Signed-off-by: Artur Niederfahrenhorst [email protected]
Why are these changes needed?
Replaces imports of pygame with try_imports, akin to the ones we use for frameworks.
This helps us output a more informative import error.
Related issue number
#31321
Checks
git commit -s) in this PR.scripts/format.shto lint the changes in this PR.