API docs for fields.Field.allow_none and fields.Field.load_default are slightly confusing / wrong

[grmpfhmbl]

I find the API docs for allow_none and load_default in the fields.Field class slightly confusing / incomplete.

The docs state for allow_none that it defaults to True if it's unset and load_default is None

marshmallow/src/marshmallow/fields.py

Lines 99 to 101 in 1ddf058

	:param allow_none: Set this to `True` if `None` should be considered a valid value during
	validation/deserialization. If ``load_default=None`` and ``allow_none`` is unset,
	will default to ``True``. Otherwise, the default is ``False``.

This kind of contradicts the docs (and as I later found type hints) of load_default. It mentions no default value but states it may be a value or callable - nothing about it being None. I then (wrongly) assumed it defaults to None and that if neither of the parameters are provided allow_none will default to True.

marshmallow/src/marshmallow/fields.py

Lines 83 to 84 in 1ddf058

	:param load_default: Default deserialization value for the field if the field is not
	found in the input data. May be a value or a callable.

Turns out that assumption was wrong. I got an error message "Field may not be null." and had to read the code to understand that load_default defaults to util.Missing(). On explicitly setting allow_none = True when creating Field() everything worked as expected.

My suggestion is to explicitly mention in the docs for allow_none that it will default to False (or None?) when neither parameter is set and mention the default value of load_default in its docs. Alternatively / additionally you might want to either change the type hint on load_default to include None, or change

marshmallow/src/marshmallow/fields.py

Line 210 in 1ddf058

self.allow_none = load_default is None if allow_none is None else allow_none

to include the load_default == missing_ case. But that probably changes the behaviour of the class in a breaking way, so I'm not sure if you want to opt for that.

[lafrech]

The type hint on load_default says Any. I'm not sure what we can change here.

The logic is that allow_none defaults to False unless load_default is None, in which case allow_none defaults to True (because the intent of the user is obviously to allow None in this case).

Perhaps the docstring for allow_none could be rephrased this way, with the usual case first and that specific case mentioned as an exception.

Overall, I think the doc is fine. What tricked you is that load_default does not default to None but to the internal missing singleton. But you might not have gone that far if the docstring for allow_none was clearer, I suppose.

[grmpfhmbl]

The type hint on load_default says Any. I'm not sure what we can change here.

Ups... my apologies. Not sure where I was looking when I wrote the type hint sentence. :-(

Overall, I think the doc is fine. What tricked you is that load_default does not default to None but to the internal missing singleton. But you might not have gone that far if the docstring for allow_none was clearer, I suppose.

Very likely. If that pitfall was mentioned at allow_none I likely would have set it to True explicitly right away.