fix: make _CharmSpec covariant in its charm type - #2715
Merged
tonyandrewmeyer merged 3 commits intoSep 1, 2026
Merged
Conversation
_CharmSpec[MyCharm] wasn't assignable where _CharmSpec[ops.CharmBase] was expected, because the type parameter was invariant. _CharmSpec is a frozen dataclass, so a covariant parameter is sound; use a separate covariant TypeVar rather than changing the public CharmType, which is also used in input positions on Context and Manager. Also make autoload generic, so that the spec it returns for a charm subclass carries the subclass through rather than erasing it to CharmBase.
tonyandrewmeyer
commented
Aug 31, 2026
tonyandrewmeyer
marked this pull request as ready for review
August 31, 2026 02:18
james-garner-canonical
approved these changes
Sep 1, 2026
james-garner-canonical
left a comment
james-garner-canonical
left a comment
Contributor
There was a problem hiding this comment.
Thanks, this turned out simpler than I had anticipated it might!
Comment on lines
-2300
to
+2304
| def autoload(charm_type: type[CharmBase]) -> _CharmSpec[CharmBase]: | ||
| def autoload(charm_type: type[CharmType]) -> _CharmSpec[CharmType]: |
Contributor
There was a problem hiding this comment.
I think it makes sense that this isn't the covariant type variable -- we're just passing the type through, so an invariant type variable makes sense.
Co-authored-by: James Garner <james.garner@canonical.com>
tromai
approved these changes
Sep 1, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
_CharmSpec[MyCharm]isn't assignable where_CharmSpec[ops.CharmBase]is expected, because the type parameter is invariant._CharmSpecis a frozen dataclass, so a covariant parameter is sound, and this adds a separate covariantTypeVarfor it rather than changing the publicCharmType, which is also used in input positions onContextandManagerand is exported, so flipping its variance would be a change for anyone annotating against it.autoloadis generic now too. It was typed(type[CharmBase]) -> _CharmSpec[CharmBase], so it erased the subclass even before variance came into it, which is whyContext.__init__needed a# pyright: ignore[reportAssignmentType]on the call. That ignore is gone, which is a reasonable check that the fix does what it says.James asked on the issue whether to add a
TypeVardefault at the same time, for the ergonomics in cases like #2248. I've left that out: it needstyping_extensions.TypeVarfor the Pythons we support, #2248 has since merged with aContext[Any]workaround, and it seems like a separate decision to the variance one.Fixes #2242