Skip to content
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

Fix propagating only and exclude to nested field instances #1385

Merged
merged 8 commits into from
Sep 12, 2019

Conversation

sloria
Copy link
Member

@sloria sloria commented Sep 8, 2019

Fix #1384

@sloria sloria requested a review from lafrech September 8, 2019 23:34
@@ -878,7 +875,9 @@ def __apply_nested_option(self, option_name, field_names, set_operation):
setattr(self.declared_fields[key], option_name, new_options)

def _init_fields(self):
"""Update fields based on schema options."""
"""Update self.fields, self.load_fields, and self.dump_fields based on schema options.
This method is private API.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was always private even though we don't actually use __, so I don't consider this a breaking change.

@sloria
Copy link
Member Author

sloria commented Sep 12, 2019

Merging this because I don't want this bug and the bug in #1160 in the wild for too long.

@sloria sloria merged commit d104a5c into dev Sep 12, 2019
@sloria sloria deleted the fix-only-exclude-propagation branch September 12, 2019 02:02
@@ -468,8 +468,17 @@ def schema(self):
# Inherit context from parent.
context = getattr(self.parent, "context", {})
if isinstance(self.nested, SchemaABC):
self._schema = self.nested
self._schema = copy.deepcopy(self.nested)
Copy link

@metheoryt metheoryt Sep 15, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have nested schema instance which context contains non-pickling object. In my case it is Crypto.Cipher.AES object which i tried to deepcopy in console and got an error. Should I start an issue?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it fix it if you use copy.copy instead?

Suggested change
self._schema = copy.deepcopy(self.nested)
self._schema = copy.copy(self.nested)

Now that I think of it, I'm not sure we even need to deepcopy here. Let me know if the above fixes your issue.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sloria yep, copy works just fine! waiting for release, thanks!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for checking. The fix is released in 3.1.1.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Dotted only and exclude not working for nested schema instances
2 participants