Marshmallow: Passing an incorrect method name to Method

[lassandroan]

Hi all, this question is in reference to making a typo for method fields such as:

class MySchema(Schema):
    # Method constructor argument is misspelled but actual method is not
    foobar = fields.Method("get_footbar")

    def foobar(self, obj):
         return obj.some_other_data()

It will yield an error ValueError: Object None is not callable. from the Marshmallow utility callable_or_raise():

  File ".../lib/python3.8/site-packages/marshmallow/schema.py", line 557, in dump
    result = self._serialize(processed_obj, many=many)
  File ".../lib/python3.8/site-packages/marshmallow/schema.py", line 521, in _serialize
    value = field_obj.serialize(attr_name, obj, accessor=self.get_attribute)
  File ".../lib/python3.8/site-packages/marshmallow/fields.py", line 312, in serialize
    return self._serialize(value, attr, obj, **kwargs)
  File ".../lib/python3.8/site-packages/marshmallow/fields.py", line 1663, in _serialize
    method = utils.callable_or_raise(
  File ".../lib/python3.8/site-packages/marshmallow/utils.py", line 285, in callable_or_raise
    raise ValueError("Object {!r} is not callable.".format(obj))
ValueError: Object None is not callable.

I dug a bit of digging and found that the method is being accessed from the schema class via getattr(self.parent, self.serialize_method_name, None).

Is there a reason for the None? Without it Python would be up an AttributeError with a message containing the class name and the attribute name, but with it Marshmallow simply passes up the ValueError which doesn't give much information on what went wrong.

Here is the code where it happens for reference, grabbed from the recent dev source:

https://github.com/marshmallow-code/marshmallow/blob/0689e5522638816ef8c24f1c3e9e6f9f6f15b126/src/marshmallow/fields.py#L1713-L1728

[sloria]

I think this might be a bit of legacy. I agree that we should allow the AttributeError to be raised.

Since raising an ValueError is not part of the documented API, I think we can go ahead and change this. PRs welcome!

[lassandroan]

@sloria great, thanks! I opened a PR https://github.com/marshmallow-code/marshmallow/pull/1675 for the issue

[lassandroan]

Completed via merge of https://github.com/marshmallow-code/marshmallow/pull/1675