Generate Python dataclasses and from_dict from Go status structs

[benhoyt]

Just putting this here in case others want to see it. Something like structs.py will end up as _types.py in Jubilant, after massaging it a fair bit: https://github.com/canonical/jubilant/blob/main/jubilant/_types.py

[james-garner-canonical]

I agree that frozen is a good call. I remembered just now about dataclasses.replace, which I think makes working with frozen dataclasses easy.

To support the read-only nature of the data, maybe we should use Sequence and Mapping instead of list and dict for the type annotations?

We could add a level of runtime safety by using () as the default for Sequenceattributes , and lambda: types.MappingProxyType({}) as the default_factory for Mapping attributes.

We could use tuple(...) and MappingProxyType(...) in the from_dict constructors.

Users constructing their own instances for tests would still be able to use ordinary lists and dicts when calling __init__ without any type checker complaints.

[benhoyt]

I'm okay with frozen=True as it's free, but I prefer the simple types, and list and dict match what you get from parsed JSON anyway. In my experience, it's rare to get bugs from this kind of "extra mutability" in Python, so the simpler types win. I know I'd much rather see dict[str, AppStatus] when I hover a field in my IDE than types.MappingProxyType[str, AppStatus]. Everyone knows what the former means; I would have had to look up the latter to realise "oh, it's basically a dict". That's my current take, anyway...

[james-garner-canonical]

Just a little additional info I feel compelled to share.

In a repl you'd see mappingproxy({key: value}), just like when you inspect an object's __dict__ property. The mappingproxy(...) does look uglier, so I definitely see the case for just returning a regular dict at runtime.

In your IDE I think you'd just see Mapping[str, AppStatus], whether we returned a MappingProxyType or a dict at runtime.

I do still think that seeing Mapping instead of dict in the editor is fine (and good if we're using frozen=True), but I can see why dict would be preferable. I think my preference would be Mapping annotation with dict at runtime.

---

On lists I just want to mention two ideas:

• If you decide to useMapping[str, AppStatus] annotations, with dicts at runtime, we could use Sequence for the annotation and list at runtime
• If you want to stick to simple builtin types, we could use tuple instead of list, both for type annotations and at runtime

---

No real objections if you decide to go with dict and list in all cases, just wanted to make sure all the info is out there.

[benhoyt]

Thanks for the details. I actually kind of like the idea of using Mapping[K, V] and Sequence[T] for the type annotations, and dict and list as the actual runtime type. I'll give it some more thought. @tonyandrewmeyer, what do you think about the above?

[tonyandrewmeyer]

Would dict and list be the types when you use the from_dict method, using Jubilant in the expected way, but you might have other mapping/sequence types if they were passed to __init__? Or would you always coerce to dict and list?

I also like seeing simple types (but I'm also not particularly a fan of type annotations in Python either), but I have seen people mutate something they weren't meant to - although that's often when an object is shared/cached and that doesn't seem relevant here.

Having tuples instead of lists actually sounds nice, but you'd have to provide a custom object decoder for the JSON, or post-process it, right? I'm not sure it would be worth the effort.

Overall, I do think it's cleanest (and most duck-typing-ist, so most Pythonic) to have types that describe the characteristics we require, not the ones we happen to be using. So sequence and mapping seem good to me.

Side note: for many of these (maybe all, at least in beta?) I'd suggest using kw_only as well as frozen.

[james-garner-canonical]

Re: whether __init__ should coerce user args, imo it's fine to just use Sequence and Mapping type annotations and let users pass whatever makes them / their editor happy

Re: providing tuples instead of lists, I would suggest something like this in from_dict (if we want to use tuples)

return cls(
    attr=tuple(d.get('attr', [])),
)

Just a note about kw_only -- that's python 3.10+ only, right? I forget what we decided about Python version support for jubilant

[benhoyt]

I've thought about this further, and I significantly prefer simple types for this use case (list and dict). They've been around forever and they're here to stay, they're very obvious and common, and they're 1:1 with the JSON types this data is coming from. They're also generated anew every time juju.status() is called, not kept around in a cache, so mutability is less of a concern. Keep It Stupid Simple!

However, I agree about kw_only -- I'll add that. Currently I've got the min Python version set to 3.12 for this lib, though I'm not sure I can get away with that.

[benhoyt]

Closing this in favour of the one against the main (4.0) branch, which is the one I think we want to work against: #2