tmt icon indicating copy to clipboard operation
tmt copied to clipboard

Move dataclass to dedicated module

Open LecrisUT opened this issue 1 year ago • 10 comments

A few reasons for this move:

  • It would make migrating to attrs more easy #2831
  • Plugin developers should use the same dataclass type
  • This would allow some neat customization, e.g. if we add https://github.com/rr-/docstring_parser, we can move the help to the attribute's docsting

A few things to point out:

  • All dataclass and field usage are now unified and populate the metadata. In principle this should be fine since it would just populate a field that would never be used, but should we do something else there to distinguish between DataContainer type of dataclasses and plain dataclass like the ones in hardware.py?
  • The import syntax is a bit disjoint. The naming of dataclasses and the from import is done like this in order to minimize the diffs for the purpose of initial review, but I think we should change it to be more uniform w.r.t. full name tmt.dataclasses.dataclass vs short name field

Pull Request Checklist

  • [x] implement the feature
  • [ ] write the documentation

LecrisUT avatar Sep 05 '24 16:09 LecrisUT

What do you think about putting these into a package tmt.containers and moving the remaining implementations there as well.

LecrisUT avatar Sep 05 '24 17:09 LecrisUT

What do you think about putting these into a package tmt.containers...

+1 to tmt.containers. I have it somewhere in my stash, I tried something similar: move basic classes and functions and helpers like field() or container_* out of tmt.utils. It'd be aligned with the goal of separating code from the implementation detail like the dataclasses (or attrs, when the time comes).

... and moving the remaining implementations there as well.

Which implementations? If you mean SerializableContainer and other "data container" classes, then yep, I'd like to do that as well. My idea was to have all these helpers and classes in their own module, both to simplify tmt.utils and keep the rest of the code clean.

happz avatar Sep 05 '24 19:09 happz

Cool, I've moved the stuff around, I think after this the only things left to move would be Common and the Exceptions? Otherwise everything else would still be under utils but split into more manageable files.

I think it would also be good to start using __future__.annotations and .pyi files, it's so easy to hit circular dependencies on these.

Next step for this would be to detangle the some of these dataclasses and revert some of the redirects. In some cases it is obvious like tmt.utils.CommandOutput should be vanilla dataclass, and everything inheriting container should be the redirected, but then there are some grey ones like tmt.hardware.ConstraintComponents which are a bit in between those. Any thoughts on renaming dataclass to something distinct from vanilla dataclass/attrs.define? What about tmt_data/fmf_data, and maybe similarly tmt_field/fmf_field?

LecrisUT avatar Sep 06 '24 09:09 LecrisUT

I think it would also be good to start using future.annotations

+1

martinhoyer avatar Sep 06 '24 10:09 martinhoyer

Cool, I've moved the stuff around, I think after this the only things left to move would be Common and the Exceptions? Otherwise everything else would still be under utils but split into more manageable files.

Exceptions were on my list for sure, something like tmt.exceptions (or _exceptions). I was fine with Common staying where it was, maybe being moved into some tmt.utils submodule.

I think it would also be good to start using __future__.annotations and .pyi files, it's so easy to hit circular dependencies on these.

__future__.annotations sounds good, I'm much less fond of stub files, it always felt detached from the code & two places that need to be updated when changing signatures.

Next step for this would be to detangle the some of these dataclasses and revert some of the redirects. In some cases it is obvious like tmt.utils.CommandOutput should be vanilla dataclass

It already is, isn't it?

and everything inheriting container should be the redirected, but then there are some grey ones like tmt.hardware.ConstraintComponents which are a bit in between those. Any thoughts on renaming dataclass to something distinct from vanilla dataclass/attrs.define? What about tmt_data/fmf_data, and maybe similarly tmt_field/fmf_field?

Hmm, none of the options looks tempting, sorry. Why do you want to rename them? That might help us spend a couple of productive weeks, selecting the right name :)

happz avatar Sep 06 '24 14:09 happz

__future__.annotations sounds good, I'm much less fond of stub files, it always felt detached from the code & two places that need to be updated when changing signatures.

I was thinking stub files as additive to keep things cleaner from parts like @overload, but indeed I can see it can easily get out-of-sync. There is one situation where it might be necessary, and that is for @deprecated because we cannot decorate an attribute.

Next step for this would be to detangle the some of these dataclasses and revert some of the redirects. In some cases it is obvious like tmt.utils.CommandOutput should be vanilla dataclass

It already is, isn't it?

and everything inheriting container should be the redirected, but then there are some grey ones like tmt.hardware.ConstraintComponents which are a bit in between those. Any thoughts on renaming dataclass to something distinct from vanilla dataclass/attrs.define? What about tmt_data/fmf_data, and maybe similarly tmt_field/fmf_field?

Hmm, none of the options looks tempting, sorry. Why do you want to rename them? That might help us spend a couple of productive weeks, selecting the right name :)

The idea here is that we can do a few more optimizations if we know that the dataclass is associated with a container/fmf object:

  • unify the attribute docstring and click help, which can help quite a bit for navigating the code with the IDEs
  • unify the fmf spec documentation with the python code (or if we are bold we could even generate/append the python docstring from the fmf files, but not sure if IDEs would render it)
  • generate json schema from the annotations
  • generally most other features could be handled by inheritance and __init_subclass__, but maybe some parts are easier done by overriding the dataclass instead, annotations stuff being one of them that I know of

But another main reason is for plugin developers to guarantee that any improvements made on the base tmt containers do not require any refactoring on their side.

LecrisUT avatar Sep 06 '24 16:09 LecrisUT

__future__.annotations sounds good, I'm much less fond of stub files, it always felt detached from the code & two places that need to be updated when changing signatures.

I was thinking stub files as additive to keep things cleaner from parts like @overload, but indeed I can see it can easily get out-of-sync. There is one situation where it might be necessary, and that is for @deprecated because we cannot decorate an attribute.

Hm, I could live with the current approach, something like field(..., deprecated=...).

Next step for this would be to detangle the some of these dataclasses and revert some of the redirects. In some cases it is obvious like tmt.utils.CommandOutput should be vanilla dataclass

It already is, isn't it?

and everything inheriting container should be the redirected, but then there are some grey ones like tmt.hardware.ConstraintComponents which are a bit in between those. Any thoughts on renaming dataclass to something distinct from vanilla dataclass/attrs.define? What about tmt_data/fmf_data, and maybe similarly tmt_field/fmf_field?

Hmm, none of the options looks tempting, sorry. Why do you want to rename them? That might help us spend a couple of productive weeks, selecting the right name :)

The idea here is that we can do a few more optimizations if we know that the dataclass is associated with a container/fmf object:

  • unify the attribute docstring and click help, which can help quite a bit for navigating the code with the IDEs
  • unify the fmf spec documentation with the python code (or if we are bold we could even generate/append the python docstring from the fmf files, but not sure if IDEs would render it)

WDYT about code being the source here? My idea was to make plugin code the single source of truth, and generate plugin documentation, schema, and everything from the code of the plugin. The same would apply to spec like test/plan/story fields.

  • generate json schema from the annotations

Yep, that's one of the goals :)

  • generally most other features could be handled by inheritance and __init_subclass__, but maybe some parts are easier done by overriding the dataclass instead, annotations stuff being one of them that I know of

But another main reason is for plugin developers to guarantee that any improvements made on the base tmt containers do not require any refactoring on their side.

I think we are aligned, at least in general, and what remains are the technicalities. For example, this move of "base container classes" into other own location, absolutely. What I'm less sure is whether we speak the same language when it comes to names, e.g.

Any thoughts on renaming dataclass to something distinct from vanilla dataclass/attrs.define? What about tmt_data/fmf_data, and maybe similarly tmt_field/fmf_field?

If you mean the original dataclasses.dataclass decorator, and you think we should use it in tmt code instead of "raw" @dataclasses.dataclass, then yes, I think that makes sense - for me, the benefit would be we could change the implementation of tmt.container.whateverdecorator without polluting diff with oneliners changing what decorator is used in all plugins and base. So this makes sense from this PoV. Just a couple of notes:

  • I would really like to avoid "data". I don't like "step data", I think "data" as a term is very vague, it can mean anything, depending on context, it's hard to speak about without ambiguity, and so on. I liked "container" because it seems less "data"-ish than dataclass and still represents what the class is used for, a container for, well, data. Fields. That's why I added various container_* helpers, so I for one would do something like from dataclasses import dataclass as container.
  • I like "field", it seems to be an acceptable term to both dataclasses and attrs, and it fits the namespace in my head. We have our custom implementation, once everything does from tmt.containers import field, do we even need to call it tmt_field/fmf_field? I'm not sure I understand the need for a distinct name here.
  • I'm not 100% sure we would be able to perform the transition as painlessly as I mentioned above, i.e. without polluting plugins with trivial import changes; given how entangled everything is internal, getting things perfect enough that we would just swap one implementation for another without plugins noticing is a very tall order - we may end up with some field_new or field_v2` temporarily, and I for one would be fine with that, as long as there is a good reason and a plan how to resolve it, in the next release, for example. We have some breathing space, although it would be fine to not regress in functionality too much, too often and for too long :)
  • I'd very much argue in favor of multiple PRs. While it's possible to put multiple nice commits into one PR, when the merge time comes, it's harder to squash what should be squashed, and preserve history. Also the review seems to work better when it's less changes in one PR, at least when a potential reviewer checks the PR for the first time - seeing 50 changed files may discourage even the bravest ones, despite the changes being put in nice & tidy commits.

happz avatar Sep 09 '24 08:09 happz

I was thinking stub files as additive to keep things cleaner from parts like @overload, but indeed I can see it can easily get out-of-sync. There is one situation where it might be necessary, and that is for @deprecated because we cannot decorate an attribute.

Hm, I could live with the current approach, something like field(..., deprecated=...).

I am thinking of something slightly different. For field deprecation I think that's the only way we can do it currently until decorators can be attached to attributes and annotations, which I don't really know how that would work. What I was thinking of deprecating via stub files is if a plugin has:

from tmt.utils import field

which would deprecate to point the developer to tmt.dataclasses.field instead. For most cases we can create a shim, but if we want to deprecate a constant/variable, that is significantly more challenging. We can do it with the stub files though because we can simply lie about its type in there.

  • unify the fmf spec documentation with the python code (or if we are bold we could even generate/append the python docstring from the fmf files, but not sure if IDEs would render it)

WDYT about code being the source here? My idea was to make plugin code the single source of truth, and generate plugin documentation, schema, and everything from the code of the plugin. The same would apply to spec like test/plan/story fields.

:+1: to that. Would work similar with the json schema generation.

  • generate json schema from the annotations

Yep, that's one of the goals :)

Here is some reference form scikit-build-core. I think we can simplify it a lot with jinja templating.

  • generally most other features could be handled by inheritance and __init_subclass__, but maybe some parts are easier done by overriding the dataclass instead, annotations stuff being one of them that I know of

But another main reason is for plugin developers to guarantee that any improvements made on the base tmt containers do not require any refactoring on their side.

I think we are aligned, at least in general, and what remains are the technicalities. For example, this move of "base container classes" into other own location, absolutely. What I'm less sure is whether we speak the same language when it comes to names, e.g.

Any thoughts on renaming dataclass to something distinct from vanilla dataclass/attrs.define? What about tmt_data/fmf_data, and maybe similarly tmt_field/fmf_field?

If you mean the original dataclasses.dataclass decorator, and you think we should use it in tmt code instead of "raw" @dataclasses.dataclass, then yes, I think that makes sense - for me, the benefit would be we could change the implementation of tmt.container.whateverdecorator without polluting diff with oneliners changing what decorator is used in all plugins and base. So this makes sense from this PoV.

Indeed we are in sync on this thought :+1:.

Just a couple of notes:

* I would really like to avoid "data". I don't like "step data", I think "data" as a term is very vague, it can mean anything, depending on context, it's hard to speak about without ambiguity, and so on. I liked "container" because it seems less "data"-ish than `dataclass` and still represents what the class is used for, a container for, well, data. Fields. That's why I added various `container_*` helpers, so I for one would do something like `from dataclasses import dataclass as container`.

Hmm, makes sense. One alternative is attrs syntax of define, wdyt? I like the looks of tmt.containers.define much better than dataclass's snytax.

* I like "field", it seems to be an acceptable term to both dataclasses and attrs, and it fits the namespace in my head. We have our custom implementation, once everything does `from tmt.containers import field`, do we even need to call it `tmt_field`/`fmf_field`? I'm not sure I understand the need for a distinct name here.

This can be relaxed. It's mostly from navigating the tmt code and finding field() and it is not clear if it's dataclasses.field or tmt.utils.field. That of course can be fixed with ruff to request qualified names, and maybe we can ship some recommended ruff settings for plugin developing.

* I'm not 100% sure we would be able to perform the transition as painlessly as I mentioned above, i.e. without polluting plugins with trivial import changes; given how entangled everything is internal, getting things perfect enough that we would just swap one implementation for another without plugins noticing is a very tall order - we may end up with some `field_new` or field_v2` temporarily, and I for one would be fine with that, as long as there is a good reason and a plan how to resolve it, in the next release, for example. We have some breathing space, although it would be fine to not regress in functionality too much, too often and for too long :)

You mean w.r.t. review or with functionality? And I guess this is for the tmt code only, or maybe there are some other tmt plugins that I should keep in mind to preserve compatibility. If it's the former, then shouldn't it be resolved by splitting the PR into more minimal commits?

* I'd very much argue in favor of multiple PRs. While it's possible to put multiple nice commits into one PR, when the merge time comes, it's harder to squash what should be squashed, and preserve history. Also the review seems to work better when it's less changes in one PR, at least when a potential reviewer checks the PR for the first time - seeing 50 changed files may discourage even the bravest ones, despite the changes being put in nice & tidy commits.

:+1: I can work with that. Let's finish the design discussion in this PR, and then I'll open a few separate PRs by cherry-picking form here.

LecrisUT avatar Sep 09 '24 10:09 LecrisUT

I was thinking stub files as additive to keep things cleaner from parts like @overload, but indeed I can see it can easily get out-of-sync. There is one situation where it might be necessary, and that is for @deprecated because we cannot decorate an attribute.

Hm, I could live with the current approach, something like field(..., deprecated=...).

I am thinking of something slightly different. For field deprecation I think that's the only way we can do it currently until decorators can be attached to attributes and annotations, which I don't really know how that would work. What I was thinking of deprecating via stub files is if a plugin has:

from tmt.utils import field

which would deprecate to point the developer to tmt.dataclasses.field instead. For most cases we can create a shim, but if we want to deprecate a constant/variable, that is significantly more challenging. We can do it with the stub files though because we can simply lie about its type in there.

I'm not sure we speak about the same kind of deprecation. I meant one when a "metadata key" or CLI option gets deprecated from users' PoV. Like --format:

@option(
    '--format', default=_test_export_default, 
    show_default=True,
    help='Output format.',
    deprecated=Deprecated('1.21', hint='use --how instead'),
    choices=_test_export_formats)

I'm starting tot think you meant deprecating use of dataclass.field itself. Is that correct?

  • unify the fmf spec documentation with the python code (or if we are bold we could even generate/append the python docstring from the fmf files, but not sure if IDEs would render it)

WDYT about code being the source here? My idea was to make plugin code the single source of truth, and generate plugin documentation, schema, and everything from the code of the plugin. The same would apply to spec like test/plan/story fields.

👍 to that. Would work similar with the json schema generation.

  • generate json schema from the annotations

Yep, that's one of the goals :)

Here is some reference form scikit-build-core. I think we can simplify it a lot with jinja templating.

Yep, I played a bit with Jinja and plugins while creating plugin docs, I got some general feeling, and this should be definitely doable. I would very much like to add some library that would let us reliably process annotations, manually it's a PITA to distinguish between int and list[int] with all corner cases. And I think such a library would be needed for this endeavor where we want to convert dataclass field into a JSON schema describing it... Maybe there's a piece in attrs we could use for this.

  • generally most other features could be handled by inheritance and __init_subclass__, but maybe some parts are easier done by overriding the dataclass instead, annotations stuff being one of them that I know of

But another main reason is for plugin developers to guarantee that any improvements made on the base tmt containers do not require any refactoring on their side.

I think we are aligned, at least in general, and what remains are the technicalities. For example, this move of "base container classes" into other own location, absolutely. What I'm less sure is whether we speak the same language when it comes to names, e.g.

Any thoughts on renaming dataclass to something distinct from vanilla dataclass/attrs.define? What about tmt_data/fmf_data, and maybe similarly tmt_field/fmf_field?

If you mean the original dataclasses.dataclass decorator, and you think we should use it in tmt code instead of "raw" @dataclasses.dataclass, then yes, I think that makes sense - for me, the benefit would be we could change the implementation of tmt.container.whateverdecorator without polluting diff with oneliners changing what decorator is used in all plugins and base. So this makes sense from this PoV.

Indeed we are in sync on this thought 👍.

Just a couple of notes:

* I would really like to avoid "data". I don't like "step data", I think "data" as a term is very vague, it can mean anything, depending on context, it's hard to speak about without ambiguity, and so on. I liked "container" because it seems less "data"-ish than `dataclass` and still represents what the class is used for, a container for, well, data. Fields. That's why I added various `container_*` helpers, so I for one would do something like `from dataclasses import dataclass as container`.

Hmm, makes sense. One alternative is attrs syntax of define, wdyt? I like the looks of tmt.containers.define much better than dataclass's snytax.

@container doesn't look bad either :) @define looks like a repetition to me - I am already defining a class, I know that, that's why I wrote class Foo, why do I need to @define it even more? On the other hand, @dataclass or @container seems like marking a class with its purpose, which makes more sense to me.

* I like "field", it seems to be an acceptable term to both dataclasses and attrs, and it fits the namespace in my head. We have our custom implementation, once everything does `from tmt.containers import field`, do we even need to call it `tmt_field`/`fmf_field`? I'm not sure I understand the need for a distinct name here.

This can be relaxed. It's mostly from navigating the tmt code and finding field() and it is not clear if it's dataclasses.field or tmt.utils.field. That of course can be fixed with ruff to request qualified names, and maybe we can ship some recommended ruff settings for plugin developing.

I suppose tmt.utils.field exists because dataclasses.field exists, and there is still a bit of a mess in terminology and what names are used for variables - field, key, option, sometimes field-ish name holds CLI option and so on. If we forbid dataclass.field, it should become clearer.

* I'm not 100% sure we would be able to perform the transition as painlessly as I mentioned above, i.e. without polluting plugins with trivial import changes; given how entangled everything is internal, getting things perfect enough that we would just swap one implementation for another without plugins noticing is a very tall order - we may end up with some `field_new` or field_v2` temporarily, and I for one would be fine with that, as long as there is a good reason and a plan how to resolve it, in the next release, for example. We have some breathing space, although it would be fine to not regress in functionality too much, too often and for too long :)

You mean w.r.t. review or with functionality? And I guess this is for the tmt code only, or maybe there are some other tmt plugins that I should keep in mind to preserve compatibility. If it's the former, then shouldn't it be resolved by splitting the PR into more minimal commits?

Both review and functionality. Smaller steps are easier to review, as long as there's some documented transition path that's being followed.

There are definitely out-of-tree plugins we need to care about, luckily we do run several tests on every tmt PR checking these as well, so we should get notified as soon as we break something. And often it may end up with fix of those plugins, again, if it's understood why and for how long a apparently weird change is needed, it's easier to sell and handle.

* I'd very much argue in favor of multiple PRs. While it's possible to put multiple nice commits into one PR, when the merge time comes, it's harder to squash what should be squashed, and preserve history. Also the review seems to work better when it's less changes in one PR, at least when a potential reviewer checks the PR for the first time - seeing 50 changed files may discourage even the bravest ones, despite the changes being put in nice & tidy commits.

👍 I can work with that. Let's finish the design discussion in this PR, and then I'll open a few separate PRs by cherry-picking form here.

happz avatar Sep 25 '24 19:09 happz

Adding this one to 1.38, let's see whether it can be done as a whole.

happz avatar Sep 25 '24 19:09 happz

Attempt2. This time I've separated the usage into tmt.container and vanilla dataclaess.dataclass. One that is rather ambiguous is tmt.queue.Task, wondering if that should also be covered, maybe by a different container-like? Anyway, this one might be a bit easier to review when split into these 2 commits.

  • Currently mypy is mad at me because container = dataclasses.dataclass doesn't seem to be recognized well
  • I have only copied the implementation from tmt.utils, but I think tmt.container should be further split into smaller files
  • What happened in tmt.steps.prepare:Prepare.go().DependencyCollection, why is that dataclass definition inside a function?

LecrisUT avatar Oct 09 '24 17:10 LecrisUT

Attempt2. This time I've separated the usage into tmt.container and vanilla dataclaess.dataclass. One that is rather ambiguous is tmt.queue.Task, wondering if that should also be covered, maybe by a different container-like? Anyway, this one might be a bit easier to review when split into these 2 commits.

I'll definitely check it out before I'm done today. It looks like a good candidate for 1.38.

  • Currently mypy is mad at me because container = dataclasses.dataclass doesn't seem to be recognized well

I think mypy even has some special handling for dataclasses, the assignment might break it, yada yada, we'll end up with a hopefully temporary # type: ignore...

  • I have only copied the implementation from tmt.utils, but I think tmt.container should be further split into smaller files

Could be, although I for one would be happy by the move alone, more granularity might make things harder to follow again. It's an extreme example now, I'd like to avoid swinging to the opposite side of granularity scale :) But let's see.

  • What happened in tmt.steps.prepare:Prepare.go().DependencyCollection, why is that dataclass definition inside a function?

Nothing serious, it's a class that's used only in that method, so I made it a nested one. It can be there, it doesn't have to be there, it's up to us. It's probably the only one in tmt codebase, I suppose it could be argued that it should move out of the method.

happz avatar Oct 11 '24 05:10 happz

Already marked as a must-have, proposed for now for 1.40. Let's try to rebase on the latest main and finish until next Friday, if possible.

psss avatar Nov 26 '24 10:11 psss

@happz please find out what to do with this if we can make it in the release

thrix avatar Jan 14 '25 10:01 thrix

@happz please find out what to do with this if we can make it in the release

Most likely not, despite the priority | must. I'm working on the rebase now - yeah, I could ping @LecrisUT, and he would do it, but I felt it's been delayed so many times I should invest some work into the progress myself. Unfortunately, pre-commit reports many type-related issues that need to be sorted out, I'll try to work on them tomorrow, but I'm afraid it won't be a one-liner fix. But I would like to get it merged, perhaps first thing in 1.43 so it gets a nice review and more testing during the 1.43 development window?

happz avatar Jan 23 '25 16:01 happz

But I would like to get it merged, perhaps first thing in 1.43 so it gets a nice review and more testing during the 1.43 development window?

That sounds reasonable to me.

psss avatar Jan 24 '25 08:01 psss

But I would like to get it merged, perhaps first thing in 1.43 so it gets a nice review and more testing during the 1.43 development window?

That sounds reasonable to me.

moved to 1.43

thrix avatar Jan 27 '25 19:01 thrix

Attempt2. This time I've separated the usage into tmt.container and vanilla dataclaess.dataclass. One that is rather ambiguous is tmt.queue.Task, wondering if that should also be covered, maybe by a different container-like? Anyway, this one might be a bit easier to review when split into these 2 commits.

That explains why there were @dataclass invocations left in the code. I got rid of them, replacing them with @container, because even Task is a container. I thought having one name for both "container" and "also container but with some methods" would be easiest. It's true our field() was designed to define plugin configuration keys or metadata fields, not a mere dataclass field, and it might be nice to have a simple field() and a more fmf/cli/plugin-focused field_but_for_plugin_step_data() in the future, but two versions of @container (or keeping @dataclass around) seems like too much decorators.

I'll definitely check it out before I'm done today. It looks like a good candidate for 1.38.

Obviously, I didn't...

  • Currently mypy is mad at me because container = dataclasses.dataclass doesn't seem to be recognized well

I think mypy even has some special handling for dataclasses, the assignment might break it, yada yada, we'll end up with a hopefully temporary # type: ignore...

Correct guess, I fixed that in tmt.container. from dataclasses import dataclass as container is the simplest, we may need to check dataclass_transform later, but maybe attrs will land soon. Now the @container is correctly detected as an alias of dataclass.

@LecrisUT all in all, I took care of the rebase, replaced the remaining @dataclass uses, and convinced mypy and pyright we know what we're doing. Let me know what do you think, I for one would consider this a good starting point for the moving-and-eventually-replacing-with-attrs project.

happz avatar Jan 30 '25 12:01 happz

I think for now to get the first part of splitting the container logic to its own package, should be ok. We can workshop the requirements more in the attrs PR.

LecrisUT avatar Jan 30 '25 13:01 LecrisUT

I think for now to get the first part of splitting the container logic to its own package, should be ok. We can workshop the requirements more in the attrs PR.

And, in the meantime, Pydantic sneaked into tmt.config... That will also need some amending.

IIUIC, it should be possible to get the same work from attrs, mostly after reading https://threeofwands.com/why-i-use-attrs-instead-of-pydantic/ recently - easier to extend, compose, and customize, Pydantic seems to provide a lot of functionality that, if not needed, one needs to opt-out or even cannot disable.

happz avatar Jan 30 '25 13:01 happz

/packit build

happz avatar Feb 06 '25 12:02 happz

/packit build

happz avatar Feb 07 '25 10:02 happz

/packit build

psss avatar Feb 07 '25 12:02 psss

/packit test

psss avatar Feb 10 '25 09:02 psss

Are we sure about the name container? I don't have a better one myself, just raising it as I imagine it could clash with lxc nomenclature.

Better naming for sure would be good. Right now there is the tree nomenclature for everything that has to do with the fmf path, but we would need something for the yaml contents on each branch/leaf.

LecrisUT avatar Feb 10 '25 12:02 LecrisUT

/packit build

happz avatar Feb 10 '25 12:02 happz

Are we sure about the name container? I don't have a better one myself, just raising it as I imagine it could clash with lxc nomenclature.

Better naming for sure would be good. Right now there is the tree nomenclature for everything that has to do with the fmf path, but we would need something for the yaml contents on each branch/leaf.

So the base class is DataContainer, we could use datacontainer instead of container but I'd say these double-word names do not look very nice. I'd suggest to keep container for now, not a big deal, we can rename later if a better idea comes.

psss avatar Feb 10 '25 13:02 psss

Are we sure about the name container? I don't have a better one myself, just raising it as I imagine it could clash with lxc nomenclature.

Better naming for sure would be good. Right now there is the tree nomenclature for everything that has to do with the fmf path, but we would need something for the yaml contents on each branch/leaf.

So the base class is DataContainer, we could use datacontainer instead of container but I'd say these double-word names do not look very nice. I'd suggest to keep container for now, not a big deal, we can rename later if a better idea comes.

FTR, DataContainer may disappear. It was my addition to having a unified base for classes that serve as containers for data, to replace dictionaries, and provide some shared functionality - converting to dictionaries, iterating over fields, and so on. And we may get that "for free" with some clever usage and setup of attrs or Pydantic. We'll see.

happz avatar Feb 10 '25 13:02 happz

Added label as this broke out-of-the tree tmt plugin

lukaszachy avatar Feb 28 '25 08:02 lukaszachy