From 4fdf004c67884819810ad658fccec8d240fed31e Mon Sep 17 00:00:00 2001 From: David Ackerman <145808634+dm-ackerman@users.noreply.github.com> Date: Mon, 6 May 2024 19:21:52 +0000 Subject: [PATCH 1/7] Remove items from OrderEnforcing __getattr__ The following were removed: value == "unwrapped" - will never be triggered because the base class has an unwrapped property which returns the same result value == "render_mode" - duplicates the effect of the base class's __getattr__. value == "possible_agents", "observation_spaces", "action_spaces", and "agent_order" - not related to the stated goal of the class --- pettingzoo/utils/wrappers/order_enforcing.py | 28 ++------------------ 1 file changed, 2 insertions(+), 26 deletions(-) diff --git a/pettingzoo/utils/wrappers/order_enforcing.py b/pettingzoo/utils/wrappers/order_enforcing.py index 649c23caa..a8bf08803 100644 --- a/pettingzoo/utils/wrappers/order_enforcing.py +++ b/pettingzoo/utils/wrappers/order_enforcing.py @@ -36,32 +36,8 @@ def __init__(self, env: AECEnv[AgentID, ObsType, ActionType]): super().__init__(env) def __getattr__(self, value: str) -> Any: - """Raises an error message when data is gotten from the env. - - Should only be gotten after reset - """ - if value == "unwrapped": - return self.env.unwrapped - elif value == "render_mode" and hasattr(self.env, "render_mode"): - return self.env.render_mode # pyright: ignore[reportGeneralTypeIssues] - elif value == "possible_agents": - try: - return self.env.possible_agents - except AttributeError: - EnvLogger.error_possible_agents_attribute_missing("possible_agents") - elif value == "observation_spaces": - raise AttributeError( - "The base environment does not have an possible_agents attribute. Use the environments `observation_space` method instead" - ) - elif value == "action_spaces": - raise AttributeError( - "The base environment does not have an possible_agents attribute. Use the environments `action_space` method instead" - ) - elif value == "agent_order": - raise AttributeError( - "agent_order has been removed from the API. Please consider using agent_iter instead." - ) - elif ( + """Raises an error if certain data is accessed before reset.""" + if ( value in { "rewards", From a39366e7e7486572799ce14fd33a40b51ce466fe Mon Sep 17 00:00:00 2001 From: David Ackerman <145808634+dm-ackerman@users.noreply.github.com> Date: Mon, 6 May 2024 19:24:00 +0000 Subject: [PATCH 2/7] Correct OrderEnforcingWrapper docstring The previous one did not accurately reflect the class behavior. --- pettingzoo/utils/wrappers/order_enforcing.py | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/pettingzoo/utils/wrappers/order_enforcing.py b/pettingzoo/utils/wrappers/order_enforcing.py index a8bf08803..a766a3a90 100644 --- a/pettingzoo/utils/wrappers/order_enforcing.py +++ b/pettingzoo/utils/wrappers/order_enforcing.py @@ -19,11 +19,13 @@ class OrderEnforcingWrapper(BaseWrapper[AgentID, ObsType, ActionType]): """Checks if function calls or attribute access are in a disallowed order. - * error on getting rewards, terminations, truncations, infos, agent_selection before reset - * error on calling step, observe before reset - * error on iterating without stepping or resetting environment. - * warn on calling close before render or reset - * warn on calling step after environment is terminated or truncated + The following are raised: + * AttributeError if any of the following are accessed before reset(): + rewards, terminations, truncations, infos, agent_selection, + num_agents, agents. + * An error if any of the following are called before reset: + render(), step(), observe(), state(), agent_iter() + * A warning if step() is called when there are no agents remaining. """ def __init__(self, env: AECEnv[AgentID, ObsType, ActionType]): From efe36d00ed7b04df9400251c81c83771aad8d0f9 Mon Sep 17 00:00:00 2001 From: David Ackerman <145808634+dm-ackerman@users.noreply.github.com> Date: Mon, 6 May 2024 19:25:23 +0000 Subject: [PATCH 3/7] Remove unused variable from OrderEnforcingWrapper _has_rendered was never used --- pettingzoo/utils/wrappers/order_enforcing.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/pettingzoo/utils/wrappers/order_enforcing.py b/pettingzoo/utils/wrappers/order_enforcing.py index a766a3a90..9f74f7f8f 100644 --- a/pettingzoo/utils/wrappers/order_enforcing.py +++ b/pettingzoo/utils/wrappers/order_enforcing.py @@ -33,7 +33,6 @@ def __init__(self, env: AECEnv[AgentID, ObsType, ActionType]): env, AECEnv ), "OrderEnforcingWrapper is only compatible with AEC environments" self._has_reset = False - self._has_rendered = False self._has_updated = False super().__init__(env) @@ -59,7 +58,6 @@ def __getattr__(self, value: str) -> Any: def render(self) -> None | np.ndarray | str | list: if not self._has_reset: EnvLogger.error_render_before_reset() - self._has_rendered = True return super().render() def step(self, action: ActionType) -> None: From 62ee638860afbd3118d7044ec9eead542a8ed150 Mon Sep 17 00:00:00 2001 From: David Ackerman <145808634+dm-ackerman@users.noreply.github.com> Date: Mon, 6 May 2024 19:27:44 +0000 Subject: [PATCH 4/7] Remove extra else/return in OrderEnforcingWrapper --- pettingzoo/utils/wrappers/order_enforcing.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/pettingzoo/utils/wrappers/order_enforcing.py b/pettingzoo/utils/wrappers/order_enforcing.py index 9f74f7f8f..43f3a0c69 100644 --- a/pettingzoo/utils/wrappers/order_enforcing.py +++ b/pettingzoo/utils/wrappers/order_enforcing.py @@ -52,8 +52,7 @@ def __getattr__(self, value: str) -> Any: and not self._has_reset ): raise AttributeError(f"{value} cannot be accessed before reset") - else: - return super().__getattr__(value) + return super().__getattr__(value) def render(self) -> None | np.ndarray | str | list: if not self._has_reset: @@ -66,7 +65,6 @@ def step(self, action: ActionType) -> None: elif not self.agents: self._has_updated = True EnvLogger.warn_step_after_terminated_truncated() - return None else: self._has_updated = True super().step(action) @@ -100,8 +98,7 @@ def __str__(self) -> str: if self.__class__ is OrderEnforcingWrapper else f"{type(self).__name__}<{str(self.env)}>" ) - else: - return repr(self) + return repr(self) class AECOrderEnforcingIterable(AECIterable[AgentID, ObsType, ActionType]): From cf83ab10d723cebe8e7ee39ff9ccbbbe52634383 Mon Sep 17 00:00:00 2001 From: David Ackerman <145808634+dm-ackerman@users.noreply.github.com> Date: Mon, 6 May 2024 19:28:46 +0000 Subject: [PATCH 5/7] Fix pyright errors in AECOrderEnforcingIterator The problem stemmed from the iterator being based on AECEnv but accessing env._has_updated which only exists in the OrderEnforcingWrapper. Pyright's error is correct based on the types given in the code. However, the usage of the class is correct. This is a workaround that uses a more precise type and convinces pyright that the code is correct. --- pettingzoo/utils/wrappers/order_enforcing.py | 22 +++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/pettingzoo/utils/wrappers/order_enforcing.py b/pettingzoo/utils/wrappers/order_enforcing.py index 43f3a0c69..cfae41ccf 100644 --- a/pettingzoo/utils/wrappers/order_enforcing.py +++ b/pettingzoo/utils/wrappers/order_enforcing.py @@ -107,13 +107,25 @@ def __iter__(self) -> AECOrderEnforcingIterator[AgentID, ObsType, ActionType]: class AECOrderEnforcingIterator(AECIterator[AgentID, ObsType, ActionType]): - def __next__(self) -> AgentID: - agent = super().__next__() + def __init__( + self, env: OrderEnforcingWrapper[AgentID, ObsType, ActionType], max_iter: int + ): assert hasattr( - self.env, "_has_updated" + env, "_has_updated" ), "env must be wrapped by OrderEnforcingWrapper" + # this is set during the super call to init, so setting it here + # is redundant. However, it silences pyright errors because it tells + # pyright that self.env is an OrderEnforcingWrapper (which may not be + # strictly true, but it should have OrderEnforcingWrapper somewhere + # in the wrapper list). This might be better handled by Protocols, + # but this approach works. + self.env = env # silence pyright errors + super().__init__(env, max_iter) + + def __next__(self) -> AgentID: + agent = super().__next__() assert ( - self.env._has_updated # pyright: ignore[reportGeneralTypeIssues] + self.env._has_updated ), "need to call step() or reset() in a loop over `agent_iter`" - self.env._has_updated = False # pyright: ignore[reportGeneralTypeIssues] + self.env._has_updated = False return agent From 5bfe998e20c071cc1a6457d06fcba2019c1961fa Mon Sep 17 00:00:00 2001 From: David Ackerman <145808634+dm-ackerman@users.noreply.github.com> Date: Mon, 6 May 2024 19:53:22 +0000 Subject: [PATCH 6/7] Remove unused warnings from EnvLogger --- pettingzoo/utils/env_logger.py | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/pettingzoo/utils/env_logger.py b/pettingzoo/utils/env_logger.py index c5e640e47..bd505e2e3 100644 --- a/pettingzoo/utils/env_logger.py +++ b/pettingzoo/utils/env_logger.py @@ -61,20 +61,6 @@ def warn_action_out_of_bound( f"[WARNING]: Received an action {action} that was outside action space {action_space}. Environment is {backup_policy}" ) - @staticmethod - def warn_close_unrendered_env() -> None: - """Warns: ``[WARNING]: Called close on an unrendered environment.``.""" - EnvLogger._generic_warning( - "[WARNING]: Called close on an unrendered environment." - ) - - @staticmethod - def warn_close_before_reset() -> None: - """Warns: ``[WARNING]: reset() needs to be called before close.``.""" - EnvLogger._generic_warning( - "[WARNING]: reset() needs to be called before close." - ) - @staticmethod def warn_on_illegal_move() -> None: """Warns: ``[WARNING]: Illegal move made, game terminating with current player losing.``.""" From 12a9d63fae70a68e750c7883879c024c3fb0ba26 Mon Sep 17 00:00:00 2001 From: David Ackerman <145808634+dm-ackerman@users.noreply.github.com> Date: Tue, 7 May 2024 22:23:54 +0000 Subject: [PATCH 7/7] Ignore pyright error --- pettingzoo/utils/wrappers/order_enforcing.py | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/pettingzoo/utils/wrappers/order_enforcing.py b/pettingzoo/utils/wrappers/order_enforcing.py index cfae41ccf..4a1255682 100644 --- a/pettingzoo/utils/wrappers/order_enforcing.py +++ b/pettingzoo/utils/wrappers/order_enforcing.py @@ -110,22 +110,15 @@ class AECOrderEnforcingIterator(AECIterator[AgentID, ObsType, ActionType]): def __init__( self, env: OrderEnforcingWrapper[AgentID, ObsType, ActionType], max_iter: int ): - assert hasattr( - env, "_has_updated" + assert isinstance( + env, OrderEnforcingWrapper ), "env must be wrapped by OrderEnforcingWrapper" - # this is set during the super call to init, so setting it here - # is redundant. However, it silences pyright errors because it tells - # pyright that self.env is an OrderEnforcingWrapper (which may not be - # strictly true, but it should have OrderEnforcingWrapper somewhere - # in the wrapper list). This might be better handled by Protocols, - # but this approach works. - self.env = env # silence pyright errors super().__init__(env, max_iter) def __next__(self) -> AgentID: agent = super().__next__() assert ( - self.env._has_updated + self.env._has_updated # pyright: ignore[reportGeneralTypeIssues] ), "need to call step() or reset() in a loop over `agent_iter`" - self.env._has_updated = False + self.env._has_updated = False # pyright: ignore[reportGeneralTypeIssues] return agent