Wizkit support #78
Wizkit support #78
Conversation
facebook-github-bot
added
the
CLA Signed
nle/env/base.py
Outdated
| @@ -348,7 +352,7 @@ def _in_moveloop(self, observation): | |||
| program_state = observation[self._program_state_index] | |||
| return program_state[3] # in_moveloop | |||
|
|
|||
| def reset(self): | |||
| def reset(self, *args, **kwargs): | |||
There was a problem hiding this comment.
I would slightly prefer not to *args, **kwargs this but to spell out whatever legal argument we expect -- in this case, only wizkid?
| @@ -269,6 +269,10 @@ def _get_observation(self, observation): | |||
| for key, i in zip(self._original_observation_keys, self._original_indices) | |||
| } | |||
|
|
|||
| def _print_action_meanings(self): | |||
| for a_idx, a in enumerate(self._actions): | |||
| print(a_idx, a) | |||
There was a problem hiding this comment.
We have something similar when you play via python nle/scripts/play.py, but if you find this helpful, why not.
Generally we try to use _ for pseudo-private methods, so this might be better w/o the underscore?
The same goes for tests btw -- we are not 100% strict about this, but I'd prefer there to be no access to underscored variables/functions in tests, as this breaks encapsulation.
There was a problem hiding this comment.
Removed the underscore from the method's name.
How do you write unit tests for some internal behaviour of an object without accessing internal fields?
There was a problem hiding this comment.
Thanks!
That's a great question and as I said we're not 100% strict about that. But my idea would be that unit tests should test public behavior, not their private implementation.
More on this idea here: https://testing.googleblog.com/2015/01/testing-on-toilet-prefer-testing-public.html?m=1
This PR enables wizkit. To use, we need to pass a list of items to the env.reset(wizkit=list_of_items). Tests added.