Changed .shape to be a property

[RedTachyon]

In relation to issue #1880, it's similar (but not exactly the same) as what the OP there suggested. Basically the shape is now stored as self._shape, and has a @property-based getter. There is no setter, as changing the shape might also require changing low and high, and at that point it's probably better to just create a new object.

It shouldn't change the behavior at all, except that there's an exception thrown if someone tries to manually change self.shape.

I opted for a _name naming because dunder methods (as suggested by OP) are typically reserved for other stuff.

It'd be good if there's another set of eyes to see if it for sure doesn't break anything. It's simple enough, and the tests (should) pass, but

[RedTachyon]

I'm slightly confused by the tests failing, it worked on my fork, and the error seems to happen in some seemingly unrelated video recording tests.

[jkterry1]

Yep, @vwxyzjn needs to fix a flakey test he accidentally added. He said he'll do it in the morning

[jkterry1]

I'm probably missing something, but how does your PR resolve this from Ananth's issue?

We also need to patch up the base class in space.py. Change

self.shape = None if shape is None else tuple(shape)
as

if not hasattr(self, "shape") or self.shape is None:
    self.shape = None if shape is None else tuple(shape)

[RedTachyon]

So what Ananth originally proposed was changing shape to become a @property only for Box spaces. In that case, the mentioned condition is necessary to check if the concrete space (e.g. Box) has that property, and otherwise, it will be stored as usual.

So in particular, with that approach the behavior of a Discrete space doesn't change, i.e. you have the same problem there (you can do space.shape = (5,), not get an error, but not get any expected effect)

Instead, I handled it for all spaces by turning shape into a @property in the Space class.

As far as I can tell, the only differences this makes are:

• The problem is removed from all spaces instead of just one (although arguably, in the other ones it's less important)
• If someone made a custom space where they manually do self.shape = ..., it would need to be changed. Note that this would be pretty bad practice, since the shape is handled by Space itself via inheritance.

My reasons:

• While Box uses shape the most, it is also defined for all other spaces, so the behavior should stay as consistent as possible between them
• It's (imo) cleaner than checking whether the subclass already defined shape before calling super().__init__ (and it'd get messy if the subclass first calls that, and then wants to define the shape by itself)

---

tl;dr Ananth's version was specific, and needed a general guard to make sure it doesn't break/affect other spaces. My version is general, so other spaces are automatically affected (fixed?), so that guard is not necessary.

[ahmedo42]

@RedTachyon I think the tests will pass once #2394 is merged , test is fixed there

[araffin]

This PR (and gym 0.20 release) breaks all SB3 pre-trained models 😕 🤕 (and all pickled Spaces), to be backward compatible you have to patch __setstate__:
DLR-RM/stable-baselines3#573 (comment)