Expose dataclasses.asdict

[LecrisUT]

Splitting this from the work on #4642

[gemini-code-assist]

Code Review

This pull request includes an import in __init__.py that might be re-exporting a dependency. It's recommended to avoid re-exporting dependencies from __init__.py and instead have modules import their dependencies directly. The import statement also contains a redundant alias that should be simplified if the import is kept.

[happz]

A useful tool, but I'd try to slow this PR down a bit: what's the use case? It turns a dataclass into a dictionary, what's the dictionary for? Because it's really easy to misuse such a dictionary, pass it around, add arbitrary keys at wish, with very little protection. And pass it to places where it has no place.

Therefore I was trying to avoid exposing asdict as a general tool used widely, and focus more on providing semantic-aware helpers that may be based on asdict, but would be aware of the context and usage of such a dictionary. Like to_spec() and to_serialized(). Need to print out a spec-like representation of a prepare/shell plugin? Sure, asdict would likely work, but to_spec carries the meaning, plus offers a chance to do something slightly different if necessary.

So if you feel you need asdict somewhere, I would be very interested in the use case, because it might easily take us back to the old days where dictionaries were everywhere and it was impossible to tell whether a dictionary was safe to show to user, or save into results.yaml. Not saying there is no legitimate use case for asdict, I just want us to stay away from the easy path of using it without thinking more about the dictionaries it produces. You can see my fear not just here, but in helper methods like MetadataContainer.from_fmf() - yes, it's pretty much a wrapper for model_validate, but it carries some meaning - use this when you have an fmf node in one hand, and need to get a Python class. I feel the same about the opposite direction.

[LecrisUT]

what's the use case?

See the WarningsEntry in #4642.

SpecBasedContainer does not quite fit because this is going only one way from dataclass to yaml, and it is a bit wasteful to redefine a TypedDict that is basically exactly the same thing as the dataclass. If it can be specialized for dataclasses to avoid that layer, then maybe than that would reduce the boilerplate required.

DataContainer is a close equivalent to use otherwise, but right now it is used only in SpecBasedContainer and SerializableContainer which comes to the same issue as exposing dataclasses.asdict.

[happz]

what's the use case?

See the WarningsEntry in #4642.

SpecBasedContainer does not quite fit because this is going only one way from dataclass to yaml, and it is a bit wasteful to redefine a TypedDict that is basically exactly the same thing as the dataclass. If it can be specialized for dataclasses to avoid that layer, then maybe than that would reduce the boilerplate required.

I see.

• I'd add a WarningsEntry.to_spec() method: it's true there's no expectation of reading it, but the format of the file is going to be part of tmt specification, just like results.yaml format is, which for me means to_spec() would be the right API. Recent discussion also suggests SpecBasedContainer might be better if split into two as sometimes we don't really read things back, but it still would be good to keep the notion of specification being followed.
• to_spec() can return something like _RawWarningsEntry, an alias for dict[str, Any] - the point is to be able to distinguish a dictionary as a container of arbitrary keys and values, like population of European cities, and dictionaries as containers of keys driven by actual internal objects and/or specification.

(Yeah, I know I'm making it more complicated when asdict() alone would work, but I don't really care. I don't want to go back to days of plain dictionaries and typos in keys. I want these dictionaries have a clear meaning attached to them. There shall be helpers for conversions on the border between tmt and the outside world, even if internally they are as trivial as asdict().)

[LecrisUT]

• to_spec() can return something like _RawWarningsEntry, an alias for dict[str, Any]

That's a fine compromise.

---

I have also been thinking of how attrs/cattrs/pydantic can fit in these picture, and I think the best thing we can do is nuke the whole SpecBasedContainer et. al. and call attrs/cattrs/pydantic internals. We do not have any use of the internal TypedDict, we just need it for the normalize and such right, which would be obsoleted by this, right?