It took me time to understand what is happening when I passed a ('string') instead of ('string',). Maybe it would be a good idea to raise an error or accept one unique field in the only field.
Fair point, though I'm not sure which alternative is better:
only isn't a collectiononly to be a string. What would the behavior be here? Would only='field' be equivalent to only=('field', ) or would it have different behavior, like passing only='field' to Nested?I am open to suggestions.
I would choose the option 2 because it gives flexibility to the final code. But I think it is better to mantain the simplicity of the tool, avoiding too much useless flexibility. So I would go with the option 1.
It's probably "pythonic" to be forgiving but I think it's better to force users to be explicit. And why have two ways to express the same thing? Option 1 :+1:
Just ran into the same problem with exclude='field'. I think an exception should be raised.
+1 for Option 1.
I am leaning towards option 2.2 (like passing only='field' to Nested) which plucks the attribute when only is a string.
My reasoning is that it would simplify the implementation by moving the custom only handling from the Nested field to the Schema class, and would normalize the behavior of the only parameter across interfaces.
But I think it is better to mantain the simplicity of the tool, avoiding too much useless flexibility.
This functionality is already present in the Nested field and it is useful. The simplest thing to do would be to push the logic down to the schema implementation.
Consider an API that needs to expose a flat list of user names. Currently you would have to pluck the fields out of the list of objects manually. I think this is a fundamental serialization operation that belongs in the schema implementation.
user_names_schema = UserSchema(only='name')
data, errors = user_names_schema.dumps(users, many=True)
# '["username1", "username2", "username3"]'
This is also related to loads() handling this behavior for the inverse operation (see #314).
user_names_schema.loads(data, many=True)
# [{"name": "username1"}, {"name": "username2"}, {"name": "username3"}]
This is closely related to #800, where it's being proposed that we remove the "plucking" behavior when passing a string to only.
If we move forward with that, then it probably makes sense to go with option 1 and disallow non-collection input to only.
i think it's better to be forgiving and go with option 2, rather than a runtime error
I think modulating the
Nestedfunctionality this much based on the type of a parameter is confusing.
https://github.com/marshmallow-code/marshmallow/issues/800#issuecomment-388662028
@timc13 The same argument applies here. I am in the process of removing the string handling for only in Nested in favor of a dedicated Pluck field. I'm not sure adding string handling for only is the best way to support this feature during schema instantiation. It would be nice if fields could be loaded and dumped without a schema to wrap them.
user_names_schema = fields.Pluck(UserSchema, 'name', many=True)
data, errors = user_names_schema.dumps(users)
@sloria I have been experimenting with treating schema as an dict/object field internally. This allows all fields (including non-collection types) to be used like a schema. It also eliminates the need for the many schema constructor argument in favor of wrapping the schema in a list field.
https://deckar01.gitlab.io/marsha/guide.html#deserializing-data
I suppose the argument is that this is not modulating the behavior of only, but only allowing forgiveness because it is easy to pass in a string out of convenience when you only want one field.
This would just always transform a single string into a collection of strings.
By the way, this logic should apply to both only and exclude arguments
Went with option 1 (raise an error) in #896. Together with the upcoming Pluck field, I think it's a good approach.
Most helpful comment
This is closely related to #800, where it's being proposed that we remove the "plucking" behavior when passing a string to
only.If we move forward with that, then it probably makes sense to go with option 1 and disallow non-collection input to
only.