[1.10.x] Properly encode model and dataclass default for schema

[Bobronium]

Closes #4774

Fix was pretty straightforward, I guess we can have this in 1.10.x.

[Bobronium]

CI seems to be failing with installation error unrelated to the changes.

[hramezani]

I created #4782 to fix the CI problem.
I am not sure about the solution

[Bobronium]

Perhaps, I can rebase Bobronium:1.10.X-fix-schema-model-default on hramezani:setuptools?

[hramezani]

FYI, I closed #4782 because it was on main and seems not a proper fix

[hramezani]

As the problem got fixed in 78ca710, you can rebase your patch now!

[Bobronium]

Please review :)

[hramezani]

Do we need this change here?

[Bobronium]

Yes. Notice the change above: we transform models and dataclasses to dict in first if and need this to be a separate one to work.

[Bobronium]

Any ideas how to make this more explicit and obvious, btw?

[samuelcolvin]

maybe just a comment with the above explanation.

[hramezani]

I would add the first if as a separate if and move the isinstance(dft, dict) check to the top.

Here is my suggestion(I haven't tested it locally):

def encode_default(dft: Any) -> Any:
    from .main import BaseModel

    if isinstance(dft, BaseModel) or is_dataclass(dft):
        dft = cast('dict[str, Any]', pydantic_encoder(dft))

    if isinstance(dft, dict):
        return {encode_default(k): encode_default(v) for k, v in dft.items()}
    elif isinstance(dft, Enum):
        return dft.value
    elif isinstance(dft, (int, float, str)):
        return dft
    elif isinstance(dft, (list, tuple)):
        t = dft.__class__
        seq_args = (encode_default(v) for v in dft)
        return t(*seq_args) if is_namedtuple(t) else t(seq_args)
    elif dft is None:
        return None
    else:
        return pydantic_encoder(dft)

[Bobronium]

Happy to accept suggestions on phrasing as well :)

Previous suggestion

Suggested change

	if isinstance(dft, dict):
	# we can end up here either from first condition or if none of above conditions were met
	if isinstance(dft, dict):

[Bobronium]

Thanks for suggestion, @hramezani, I like your approach! I think I was hesitant on reordering at first, but it seems like the best move here, indeed.

[hramezani]

@Bobronium I left a suggestion and asked a question.

Please update

[Bobronium]

Nice catch with suggestion, @hramezani! I answered the question as well.

Please review.

[samuelcolvin]

Otherwise LGTM.

[samuelcolvin]

maybe just a comment with the above explanation.

[hramezani]

LGTM 👍
Thanks @Bobronium

[samuelcolvin]

Thanks so much.

I've also cherry-picked this into main, mostly so we don't forget this edge case when working on JSONSchema for V2.