- 
          
- 
                Notifications
    You must be signed in to change notification settings 
- Fork 646
Add missing_values parameter to Field #1381
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
base: dev
Are you sure you want to change the base?
Conversation
| with pytest.raises(ValidationError, match="required"): | ||
| ArtistSchema().load({"album_names": []}) | ||
|  | ||
| def test_setting_default_missing_values(self, monkeypatch): | 
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.
This test might not be necessary--it's sorta just testing the Python language. Just wanted to make sure this use case was met.
1ae58a2    to
    773d5d7      
    Compare
  
    Allows specifying which values are treated as "missing". Addresses #713
773d5d7    to
    afda7cb      
    Compare
  
    | LGTM. 
 What would that do? I'm tempted to say no. Do we do anything specific with  | 
| It would make  from marshmallow import Schema, fields
class ArtistSchema(Schema):
    name = fields.Str(default="---", missing_values=("",))
sch = ArtistSchema()
print(sch.dump({}))   # =>  {'name': '---'}
print(sch.dump({"name": ""})) # => {'name': ''} now, but maybe should be same as aboveSure, it's not a common use case, but it seems inconsistent to not handle it. | 
| OK, I get it. Maybe it shouldn't be the same parameter. IIUC, the primary use case for this is an API for which it is complicated to enforce removal of empty-ish values. There is no reason the same constraint would apply to the serialized form of the data. But the database may have its own constraint. When using SQL, I already felt the need to treat  Symmetry would recommend  | 
| That's a good point; the client inputs for "no data" may be different from the database's representation. 
 | 
Proves the concept proposed in #713 (comment)
Still needs docs and details, but this is ready for review.
TODO:
Respectsee discussionmissing_valueson serialization?Field.__repr__to includemissing_values