v3 no longer accepts fields that call class methods

[acelletti]

While upgrading from v2.21.0 to v3.10.0, I have found a difference in behaviour that I could not find in the Changelog / update docs.

I have created a contrived case to show the issue:

import marshmallow

class Test(marshmallow.Schema):
    number = marshmallow.fields.Number()

class TestObject(object):
	def number(self):
		return 1

if marshmallow.__version__.startswith("3"):
	print(Test().dump(TestObject()))
else:
	print(Test().dump(TestObject()).data)

When run in marshmallow==2.21.0, the output is:

{'number': 1.0}

while when run in marshmallow==3.10.0, I get this error:

TypeError: float() argument must be a string or a number, not 'method'

Can anybody shed some light on this difference in behaviour? Is it to be expected or is it a bug?

If it is expected, what is the best way to deal with this for the upgrade?

[lafrech]

I don't remember whether it was meant to work in marshmallow 2 or whether it just happened to work.

I guess it works if you make that method a property.

[acelletti]

Unfortunately, this solution of turning the method into a property will not work for our project, would you agree that this is a regression @lafrech?

[lafrech]

I understand making the method a property may not be an option.

I can't tell whether this is a regression that should be documented or if it was not designed to work in marshmallow 2 either, in which case the change could be documented anyway.

[deckar01]

It was removed intentionally from v3 for #430. It looks like it didn't get documented, probably because the behavior was a regression that also wasn't documented. I believe the reason this behavior was removed is that there was no way to opt out of calling the method. The alternative is to require the schema to explicitly declare Method fields to opt into this behavior.

FWIW my stance on patching old bugs has evolved since that discussion. I don't think it's ever too late to fix a bug anymore. Having apps break after a dependency update, because they were built on top of undocumented behavior seems acceptable.

Since we decided to commit to keeping that behavior in v2, we should probably add it to the v2 docs and upgrade guide.