added the pypiwin32 package #1668
Conversation
|
The platform specific package is following the guidance of this doc. https://setuptools.readthedocs.io/en/latest/setuptools.html#declaring-platform-specific-dependencies Pypiwin32 is better than pywin32 due to the reason stated here: https://mail.python.org/pipermail/python-win32/2016-October/013786.html The change to test_ppo.py is due to the reason here: pytest-dev/pytest#3950 The windows doc change is inspired by this page: pypa/pip#3245 |
| @@ -42,15 +42,15 @@ def dummy_config(): | |||
|
|
|||
| @mock.patch('mlagents.envs.UnityEnvironment.executable_launcher') | |||
| @mock.patch('mlagents.envs.UnityEnvironment.get_communicator') | |||
| def test_ppo_policy_evaluate(mock_communicator, mock_launcher): | |||
| def test_ppo_policy_evaluate(mock_communicator, mock_launcher, dummy_config): | |||
There was a problem hiding this comment.
Has this been tested on both Windows and Mac ?
docs/Installation-Windows.md
Outdated
| @@ -153,6 +153,12 @@ pip install -e . | |||
| This will complete the installation of all the required Python packages to run | |||
| the ML-Agents toolkit. | |||
|
|
|||
| If you are stuck during this installation, you can try: | |||
There was a problem hiding this comment.
Maybe adding a little more description of what being "stuck" is like and add a few lines describing what --no-cache-dir does.
| @@ -37,7 +37,8 @@ | |||
| 'docopt', | |||
| 'pyyaml', | |||
| 'protobuf>=3.6,<3.7', | |||
| 'grpcio>=1.11.0,<1.12.0'], | |||
| 'grpcio>=1.11.0,<1.12.0', | |||
| 'pypiwin32==223;platform_system=="Windows"'], | |||
There was a problem hiding this comment.
Was this tested on both Mac and Windows ?
There was a problem hiding this comment.
Yes I tested this on Mac, Windows and Linux.
|
I think the documentation change could use a little more description. If the changes to setup and the tests have been tested, I see no problem with them and I think we can merge them. |
* added the pypiwin32 package * fixed the break on mac, fixed part of pytest above version 4 * added something to the windows to help unstuck people * resolved the comment
No description provided.