`attributes: TAttr = cast("TAttr", {})` is risky without `validate_defaults`

[tlambert03]

Was playing around in ome-zarr-models-py, which uses this library: specifically i was playing with the case where we don't have zarr (see #112 and ome-zarr-models/ome-zarr-models-py#280) ... and relatively quickly ran into a problem with the ome_attributes property here. Turns out it's quite easy to instantiate an invalid model, and the reason ultimately boils down to this code in GroupSpec from this repo:

class GroupSpec(NodeSpec, Generic[TAttr, TItem]):
    node_type: Literal["group"] = "group"
    attributes: TAttr = cast("TAttr", {})

yes, pydantic will validate any provided attributes object as a TAttr ... but without validate_defaults=True in the model_config, it will happily accept assign that bare dictionary to attributes if the user doesn't explicitly pass in an argument to the constructor (and, it's also not mandatory that they pass in an argument for attributes, since there is a default value)

#example:
from pydantic_zarr.v3 import GroupSpec
class MyModel(GroupSpec[int, int]):
    pass

x = MyIntModel()  # no error, but totally invalid according to the model typing
# MyIntModel(zarr_format=3, node_type='group', attributes={}, members={})

I suspect the only reason this hasn't cropped up before is that that library pretty much never instantiates models from anything other than actual (mostly valid) json documents.

suggestion

If you want to have a field that validates against a generic type, I would suggest that that field should be mandatory. In other words, using attributes: TAttr = cast("TAttr", {}) is convenient perhaps for a certain use case (specifically, a TAttr model that has no required fields ... I assume that's how it got there). but it makes it extremely easy to create an invalid model.

alternative

Alternatively, this library should always use model_config: ConfigDict = ConfigDict(validate_default=True) for any model that assigns a bare dictionary to a field based on a generic. That would at least allow subclasses to parametrize GroupSpec with a model that has no mandatory fields... (but, that is not the case over in ome-zarr-models-py, where GroupSpec is getting parametrized by BaseZarrAttrs[T], which requires an 'ome' field)

---

short of one of those two solutions, this pattern is a foot-gun that will cause lots of accidentally invalid subclass instances

[d-v-b]

yes lets fix this ASAP! I'm fine with either solution, but the first one seems simpler.

[tlambert03]

k, incoming