operator icon indicating copy to clipboard operation
operator copied to clipboard

ops[testing] State.config type, should it be dict or Mapping?

Open dimaqq opened this issue 4 months ago • 3 comments

Currently it's declared like this:

  @dataclasses.dataclass(frozen=True)
  class State(_max_posargs(0)):
      """Represents the Juju-owned portion of a unit's state.

      Roughly speaking, it wraps all hook-tool- and pebble-mediated data a charm can access in its
      lifecycle. For example, status-get will return data from `State.unit_status`, is-leader will
      return data from `State.leader`, and so on.
      """

      config: dict[str, str | int | float | bool] = dataclasses.field(
          default_factory=dict,
      )

I'm trying to use it like so and pyright is unhappy:

def caller():
    config = {"init": init}
    # reportArgumentType: Argument of type "dict[str, str]" cannot be assigned to pa...
    # Consider switching from "dict" to "Mapping" which is covariant in the value type
    do_scenario_thing(config)

def do_scenario_thing(config: dict[str, str | int | float | bool]):
    state = State(..., config=config)

But if I switch to Mapping, pyright is still unhappy:

def caller():
    config = {"init": init}
    do_scenario_thing(config)

def do_scenario_thing(config: Mapping[str, str | int | float | bool]):
    # reportArgumentType: Argument of type "Mapping[str, str | int | float | bool]" is not assignable to "dict[str, str | int | float | bool]"
    state = State(..., config=config)

I wonder if this is a fundamental issue, because a dataclass field's type is used both when:

  • dataclass is created with a value
  • dataclass field is read

dimaqq avatar Oct 28 '25 10:10 dimaqq

@james-garner-canonical what's your take?

dimaqq avatar Oct 28 '25 10:10 dimaqq

AI's take on this:

  • change dataclass field to Mapping[...]
  • in post_init, wrap the incoming data into a MappingProxyType

dimaqq avatar Oct 28 '25 11:10 dimaqq

Field that may need this treatment:

State.config
DeferredEvent.snapshot_data
Relation.remote_units_data
Relation.remote_app_data  # unless charm under test can change it?
PeerRelation.peers_data

dimaqq avatar Oct 28 '25 11:10 dimaqq