Add a 'serialized' flag to properties and don't serialize if it's false #3179
Conversation
properties_with_values() now returns only serializable properties, since it's used in contexts such as to_json() and clone() which should only use serializable stuff.
|
Is there any existing example where this is needed? |
|
Yes, it's needed to get foo_units out of the json (since it's already |
| self.assertTrue('start_angle' in json) | ||
| self.assertTrue('start_angle_units' not in json) | ||
| self.assertTrue('outer_radius' in json) | ||
| self.assertTrue('outer_radius_units' not in json) |
There was a problem hiding this comment.
This test is the reason for the PR. (though it's not just AnnularWedge affected of course, it's all DistanceSpec and AngleSpec)
There was a problem hiding this comment.
Overall I see the point of this PR. Although in the case of *_units properties, I think this is only a temporary measure and there should be a special class of property to encode them.
There was a problem hiding this comment.
I'm working on that now. I have a follow-on PR that will make it a lot easier to make _units props truly 'virtual'.
Hmm. This may actually remove the need to put serialized= kwarg on every constructor, come to think of it.
| @@ -124,13 +124,14 @@ class DeserializationError(Exception): | |||
| class Property(object): | |||
| """ Base class for all type properties. """ | |||
|
|
|||
| def __init__(self, default=None, help=None): | |||
| def __init__(self, default=None, help=None, serialized=True): | |||
There was a problem hiding this comment.
So, the default is True and later you change this with prop._serialized = False, so maybe this argument is unnecessary? If it's left, then specific property classes should be allowed to get this argument as well, so you would have to change a lot of constructors (won't be pretty).
There was a problem hiding this comment.
I realized this later, yeah. The "right thing" is to add this keyword arg to all the other constructors, I think. it sort of sucks, but.
|
superseded by #3190 |
properties_with_values() now returns only serializable properties, since
it's used in contexts such as to_json() and clone() which should only
use serializable stuff.