From 58dd2b23c06ff9cb2f1bfb54b43f8ff9ddc7d35f Mon Sep 17 00:00:00 2001 From: Alexis DUBURCQ Date: Sat, 18 Jan 2025 10:39:02 +0000 Subject: [PATCH 1/2] Improve 'set_wrapper_attr'. --- gymnasium/core.py | 38 +++++++++++++++++++------------------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/gymnasium/core.py b/gymnasium/core.py index aaf9476f7..8861ad182 100644 --- a/gymnasium/core.py +++ b/gymnasium/core.py @@ -273,9 +273,12 @@ def get_wrapper_attr(self, name: str) -> Any: """Gets the attribute `name` from the environment.""" return getattr(self, name) - def set_wrapper_attr(self, name: str, value: Any): + def set_wrapper_attr(self, name: str, value: Any, *, force: bool = True): """Sets the attribute `name` on the environment with `value`.""" - setattr(self, name, value) + if force or hasattr(self, name): + setattr(self, name, value) + else: + raise AttributeError(f"{self} has no attribute {name!r}") WrapperObsType = TypeVar("WrapperObsType") @@ -425,30 +428,27 @@ def get_wrapper_attr(self, name: str) -> Any: f"wrapper {self.class_name()} has no attribute {name!r}" ) from e - def set_wrapper_attr(self, name: str, value: Any): + def set_wrapper_attr(self, name: str, value: Any, *, force: bool = True): """Sets an attribute on this wrapper or lower environment if `name` is already defined. Args: name: The variable name value: The new variable value + force: Whether to create the attribute on this wrapper if it does not exists on the + lower environment instead of raising an exception """ - sub_env = self - - # loop through all the wrappers, checking if it has the variable name then setting it - # otherwise stripping the wrapper to check the next. - # end when the core env is reached - while isinstance(sub_env, Wrapper): - if hasattr(sub_env, name): - setattr(sub_env, name, value) - return - - sub_env = sub_env.env - - # check if the base environment has the wrapper, otherwise, we set it on the top (this) wrapper - if hasattr(sub_env, name): - setattr(sub_env, name, value) - else: + if hasattr(self, name): setattr(self, name, value) + else: + try: + self.env.set_wrapper_attr(name, value, force=False) + except AttributeError as e: + if force: + setattr(self, name, value) + else: + raise AttributeError( + f"wrapper {self.class_name()} has no attribute {name!r}" + ) from e def __str__(self): """Returns the wrapper name and the :attr:`env` representation string.""" From 3312ae115a49bb0498229336762687d4b38b3c6a Mon Sep 17 00:00:00 2001 From: Mark Towers Date: Thu, 13 Feb 2025 23:05:44 +0000 Subject: [PATCH 2/2] Update `set_wrapper_attr` to return bool --- gymnasium/core.py | 33 ++++++++++++++++++--------------- 1 file changed, 18 insertions(+), 15 deletions(-) diff --git a/gymnasium/core.py b/gymnasium/core.py index 8861ad182..aedf4cb81 100644 --- a/gymnasium/core.py +++ b/gymnasium/core.py @@ -273,12 +273,12 @@ def get_wrapper_attr(self, name: str) -> Any: """Gets the attribute `name` from the environment.""" return getattr(self, name) - def set_wrapper_attr(self, name: str, value: Any, *, force: bool = True): - """Sets the attribute `name` on the environment with `value`.""" + def set_wrapper_attr(self, name: str, value: Any, *, force: bool = True) -> bool: + """Sets the attribute `name` on the environment with `value`, see `Wrapper.set_wrapper_attr` for more info.""" if force or hasattr(self, name): setattr(self, name, value) - else: - raise AttributeError(f"{self} has no attribute {name!r}") + return True + return False WrapperObsType = TypeVar("WrapperObsType") @@ -428,27 +428,30 @@ def get_wrapper_attr(self, name: str) -> Any: f"wrapper {self.class_name()} has no attribute {name!r}" ) from e - def set_wrapper_attr(self, name: str, value: Any, *, force: bool = True): + def set_wrapper_attr(self, name: str, value: Any, *, force: bool = True) -> bool: """Sets an attribute on this wrapper or lower environment if `name` is already defined. Args: name: The variable name value: The new variable value force: Whether to create the attribute on this wrapper if it does not exists on the - lower environment instead of raising an exception + lower environment instead of raising an exception + + Returns: + If the variable has been set in this or a lower wrapper. """ if hasattr(self, name): setattr(self, name, value) + return True else: - try: - self.env.set_wrapper_attr(name, value, force=False) - except AttributeError as e: - if force: - setattr(self, name, value) - else: - raise AttributeError( - f"wrapper {self.class_name()} has no attribute {name!r}" - ) from e + already_set = self.env.set_wrapper_attr(name, value, force=False) + if already_set: + return True + elif force: + setattr(self, name, value) + return True + else: + return False def __str__(self): """Returns the wrapper name and the :attr:`env` representation string."""