-
Notifications
You must be signed in to change notification settings - Fork 7
Deprecate OR embrace CheckedSession also updating __dict__ #1185
Description
When working on #1168, I realized that CheckedSession has an oddity: it "forwards" all attribute writes to self.__dict__ (via object.__setattr__) in addition to self._objects, while normal Sessions do not.
When reworking that code, I removed the oddity because that's mostly useless... All our tests passed... Until I realized yesterday by accident that the demo model relied on that (it does not crash but one of their output file is empty 😉). It uses vars(self) to get a dict of variables from a CheckedSession 😱 instead of using .items(). So I will revert the removal of the forwarding from my fix but I still would like to clean that up some day.
Unsure if we can deprecate that cleanly. Does __getattribute__ triggers for accessing __dict__ via vars()?
An alternative to deprecating it would be to embrace that instead and store all objects only in __dict__ (and remove the _objects dict) for both Session and CheckedSession. Unsure what would be the consequences. Would we loose the ability to store invalid python identifiers (unsure it has any value -- maybe to convert any Array to a Session)? Can we still handle _meta?
This is also slightly related to #1171 and should be done at the same time.