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

Allow setting a global unknown setting #1367

Closed
charlax opened this issue Aug 26, 2019 · 13 comments
Closed

Allow setting a global unknown setting #1367

charlax opened this issue Aug 26, 2019 · 13 comments

Comments

@charlax
Copy link
Contributor

charlax commented Aug 26, 2019

The new RAISE unknown setting can make the migration pretty painful, especially for those who are just fine with EXCLUDEing unknown keys. What about making this global default a configuration setting?

Something as simple as:

import marshmallow

marshmallow.DEFAULT_UNKNOWN_BEHAVIOR = marshmallow.EXCLUDE

I know it's possible to subclass Schema, but that seems more heavy-handed.

@deckar01
Copy link
Member

deckar01 commented Aug 26, 2019

You can modify the base schema's Meta class directly and your subclasses will inherit it.

from marshmallow import Schema, fields, EXCLUDE

Schema.Meta.unknown = EXCLUDE

class Test(Schema):
    foo = fields.Str()

Test().load({'foo': 'test', 'junk': 2})
# {'foo': 'test'}

@charlax
Copy link
Contributor Author

charlax commented Aug 26, 2019

Indeed, that's just as simple. Perhaps this could be added to the docs?

@deckar01
Copy link
Member

I'm not sure this would fit well in the "Handling Unknown" section of the quick start docs. Global configuration doesn't seem to have a dedicated place in the docs either. It would probably be useful to mention the pattern in the upgrading docs.

Feature requests that are solved by applying Python language features to documented implementation details do come up regularly though. I wonder if the examples folder would be a good place to put some of these patterns?

@sloria
Copy link
Member

sloria commented Aug 26, 2019

I don't think we need to add any additional API for setting defaults. I think many users use a base class that defines their defaults.

I guess we can the suggestion above to the docs, but is it really a pattern we want to encourage?

Feel free to send a PR adding the docs, but closing the issue since the use case has been met.

@sloria sloria closed this as completed Aug 26, 2019
@sloria
Copy link
Member

sloria commented Aug 26, 2019

It would probably be useful to mention the pattern in the upgrading docs.

Yeah, that's what I was thinking. We could make it clear that it's just a stopgap.

@deckar01
Copy link
Member

deckar01 commented Aug 26, 2019

is it really a pattern we want to encourage?

Right, new users shouldn't need this, because they can easily make a custom base class.

@charlax
Copy link
Contributor Author

charlax commented Aug 26, 2019

Well, the point about documenting it is that it becomes a supported use case that should be tested. Either way, this is simple enough that it can stay as is. Let me know what you think, happy to spin up a quick PR.

@sloria
Copy link
Member

sloria commented Aug 26, 2019

I'm not convinced this is something that should be tested. It's a stopgap, after all, so I don't think it's something we want to guarantee support for. Secondly, it's just using "plain Python", i.e. it's not really a marshmallow feature.

@deckar01
Copy link
Member

Being able to configure Schema.Meta directly is a generally useful pattern that we probably don't have explicit test coverage for, but schema inheritance is a Python feature, not a marshmallow feature. The only way we would regress that functionality is if schemas stopped being declared as a subclasses, and that would be a breaking change with upgrading docs.

@anandtripathi5
Copy link

You can modify the base schema's Meta class directly and your subclasses will inherit it.

from marshmallow import Schema, fields, EXCLUDE

Schema.Meta.unknown = EXCLUDE

class Test(Schema):
    foo = fields.Str()

Test().load({'foo': 'test', 'junk': 2})
# {'foo': 'test'}

This will only be applicable in that file. I want to use it as a global configuration(In other files too).

@lafrech
Copy link
Member

lafrech commented Apr 16, 2021

This will only be applicable in that file.

I don't think so. Did you check that ?

Since marshmallow.Schema is mutated, the change should apply everywhere.

@anandtripathi5
Copy link

This will only be applicable in that file.

I don't think so. Did you check that ?

Since marshmallow.Schema is mutated, the change should apply everywhere.

Yeah crosschecked

@lafrech
Copy link
Member

lafrech commented Apr 16, 2021

It should work. Could you please provide a repro case?

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

No branches or pull requests

5 participants