Document dataclass field should not be reused

[v01dXYZ]

Documentation

The current implementation of dataclass modifies a field in place. Thereby, the following code will modify f:

from dataclasses import dataclass, field

f = field(kw_only=True)

@dataclass
class A:
    x: int = f
    y: int = f

print(f) # now f.name is filled
# furthermore, A is broken (or does not behave if we used two different field instance)

It can create issues when we have multiple fields that have the same spec and we reuse the same Field instance to set the same metadata to all of them. A class is created but it's garbage.

IMHO, in the docs, we should warn the user to NOT share any Field instance.

Linked PRs

• gh-145369: Warn against reusing dataclass field objects #145373
• gh-145369: document dataclass could possibly mutate provided field objects #145387

[ethanfurman]

It's more accurate to say that dataclass does not make a copy of the field, but that is standard Python. You would have the same type of problem with:

f = list()

class A:
    x = f
    y = f

A.y.append(1)
A.x  # -> [1]

if a doc change is made, it should point that out.

[picnixz]

I don't think this needs to be documented. If this were to be documented, that'd be the data model on the class statement or something like that if it's not already the case.

[v01dXYZ]

@picnixz I understand you reject it as it adds unwanted noise in the documentation. I was surprised though to see the field was updated by dataclass and generated a broken data class (silently). So before asking about documenting anything, I should have asked: do you think it is an expected behaviour? The example provided by @ethanfurman is relevant but a list is a container, something meant to be filled and updated, which is not the case of a field at first glance. I understand your points though, I've seen _process_class in dataclasses tracks if a name is duplicated, it would be maybe cheap to implement a check to find out if there are duplicated field instances as well.

*well sorry: the validation occurs actually in make_dataclass

[picnixz]

For me it's the expected behavior. The docs say:

To satisfy this need for additional information, you can replace the default field value with a call to the provided field() function.

If really needed, I think we can amend a bit the docs here to say that the call must be explicit. Or alternative, if you want to have the same specs:

field_specs = partial(field, ...)

class A:
    x = field_specs()
    y = field_specs()

This would be the easiest way to avoid repeating the field specs. I'm not against adding a small clarification in the paragraph itself, but I'm not really fond in adding more noise to the docs.

[mpkocher]

It can create issues when we have multiple fields that have the same spec and we reuse the same Field instance to set the same metadata to all of them.

I agree that the behavior is a bit odd and not intuitive. The field() layer does look like simple config that isn't mutated.

I think there also might be some confusion created because the type hints are simple config/metadata that can be shared.

from dataclasses import dataclass, field
from typing import Annotated

Int = Annotated[type, "IntAble type metadata"]
FieldConfig = field(metadata=dict(namespace="alpha"), kw_only=True)

def to_f():
    return field(metadata=dict(namespace="alpha"), kw_only=True)

@dataclass
class Record:
    x: Int = to_f()
    y: Int = to_f()
    z: Int = FieldConfig # do not do this.

It does seem like you should be able to share field configs similar to how type hints can be shared across field definitions. (Also, I don't think the field vs Field layer is helping here).

It's also peculiar that the dataclass dynamic machinery is taking the effort to create a proxydict/frozendict/read-only around the Field metadata dict, but it's not copying the Field instance.

In [28]: r = Record(x=1, y=2, z=3)

In [29]: fields(r)[0].metadata
Out[29]: mappingproxy({'namespace': 'alpha'})

The Field instance is very thin, perhaps it should be copied to avoid these mutable issues and improve ergonomics of the dataclass interface.

[picnixz]

Deepcopying each field is not reasonable, it will slow down the class creation and therefore the module importa for a niche case. I would rather not do any runtime changes here. We try to resuce dataclass creation time as much as possible because it affects usercode at startup in a non-negligible way.