-
-
Notifications
You must be signed in to change notification settings - Fork 631
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Inherit options #1432
base: dev
Are you sure you want to change the base?
Inherit options #1432
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,6 +18,7 @@ | |
INCLUDE, | ||
RAISE, | ||
class_registry, | ||
SchemaOpts, | ||
) | ||
from marshmallow.exceptions import ( | ||
ValidationError, | ||
|
@@ -2766,3 +2767,79 @@ class Meta: | |
dumped = OSchema().dump({"foo": 42, "bar": 24}) | ||
assert isinstance(dumped, OrderedDict) | ||
assert "bar" not in dumped | ||
|
||
|
||
class TestCombineOpts: | ||
def test_different_opts(self): | ||
""" | ||
Create two options classes, each belonging to a schema, then combine the two | ||
schemas using inheritance. When combine_opts == True, we should have both options | ||
being stored in self.opts. Otherwise, we should only get options defined in the | ||
first options class stored in self.opts. | ||
""" | ||
|
||
class Opts1(SchemaOpts): | ||
def __init__(self, meta, **kwargs): | ||
super().__init__(meta, **kwargs) | ||
self.opt_1 = getattr(meta, "opt_1", None) | ||
|
||
class Schema1(Schema): | ||
OPTIONS_CLASS = Opts1 | ||
Field1 = fields.String() | ||
|
||
class Opts2(SchemaOpts): | ||
def __init__(self, meta, **kwargs): | ||
super().__init__(meta, **kwargs) | ||
self.opt_2 = getattr(meta, "opt_2", None) | ||
|
||
class Schema2(Schema): | ||
OPTIONS_CLASS = Opts2 | ||
Field2 = fields.String() | ||
|
||
class NotCombined(Schema1, Schema2, combine_opts=False): | ||
class Meta: | ||
opt_1 = True | ||
opt_2 = True | ||
|
||
class Combined(Schema1, Schema2, combine_opts=True): | ||
class Meta: | ||
opt_1 = True | ||
opt_2 = True | ||
|
||
# Without using combine_opts, we should only get options defined by the first | ||
# options class | ||
assert NotCombined().opts.opt_1 | ||
assert not hasattr(NotCombined().opts, "opt_2") | ||
|
||
# However when we use combine_opts=True, we should get options defined by both | ||
# options classes | ||
assert Combined().opts.opt_1 | ||
assert Combined().opts.opt_2 | ||
|
||
def test_same_opts(self): | ||
""" | ||
Ensure this behaviour still works if the two parent schemas have the same opts | ||
class | ||
""" | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't get the purpose of this test. In fact, I don't understand the purpose of the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The class This particular test basically just checks that using |
||
class Opts1(SchemaOpts): | ||
def __init__(self, meta, **kwargs): | ||
super().__init__(meta, **kwargs) | ||
self.opt_1 = getattr(meta, "opt_1", None) | ||
|
||
class Schema1(Schema): | ||
OPTIONS_CLASS = Opts1 | ||
Field1 = fields.String() | ||
|
||
class Schema2(Schema): | ||
OPTIONS_CLASS = Opts1 | ||
Field2 = fields.String() | ||
|
||
class Combined(Schema1, Schema2, combine_opts=True): | ||
class Meta: | ||
opt_1 = True | ||
opt_2 = True | ||
|
||
# However when we use combine_opts=True, we should get options defined by both | ||
# options classes | ||
assert Combined().opts.opt_1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rewrite proposal:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
True, that is admittedly a lot less redundant. I'll fix it at some point