Fix ColorAlignment.custom_colors.keys() / slots type mismatch #303
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is something I am running into while working on an bigger PR (and need to resolve it) and I think it would be good to separately get a quick fix in.
In the
ColorAlignment
dataclass, the type annotation ofcustom_color
keys (int
) does not match the type of the of slots (set ofstr
(which are numeric characters)), which is used to construct the ColorAlignment instance. (The value of that field ends up not the correct type).Additionally, since this value is saved in the
hyfetch.json
, serialization for json only supports strings as keys means that the keys go into the json asstr
s. And it's involved to avoid saving the keys asstr
. There are outstanding user configs already with serialized string keys. De-serializing those configs also result instr
keys in thecustom_color
field.I think the resolution is one of the following options:
custom_colors: dict[str, int] = ()
slots
to be aset[int]
, and one of:a. Enhance the deserialization to generally recognise int str keys.
b. Add a fixup step to
ColorAlignment.from_dict
I chose solution 2b here as it is simple and it respects the apparent authorial intent that the
custom_colors
should be andict[int, int]
, which in general makes sense to me. This solution would not in any way preclude smoothly moving tostr
keys in the future if moreslot
s support is desired with for example letter based slots${cA} ${cB}
.