operator icon indicating copy to clipboard operation
operator copied to clipboard

Improve type hints for Pebble identities

Open dimaqq opened this issue 7 months ago • 0 comments

test/test_pebble.py

@@ -4030,7 +4030,7 @@ def test_local_identity_from_dict(self):

    def test_local_identity_from_dict_with_access_enum(self):
        identity = pebble.Identity.from_dict({
            'access': pebble.IdentityAccess.ADMIN,
            'access': pebble.IdentityAccess.ADMIN,  # type: ignore

Member Author @dimaqq dimaqq 4 days ago This is of potential concern to our users.

If we want them to call Identity.from_dict both with strings and enum members and have that type check strictly, we may have to do some extra work.

For now, I've decided not to adjust our public types, not within the scope of this PR anyway.

Member @tonyandrewmeyer tonyandrewmeyer 4 days ago Is this because the type checker isn't ok with a str subclass when only a small set of literals is acceptable? If the typed dict accepted a string or the enum would that fix it? That seems a safe change, and more correct. I think we should fix it now, even if not in this PR.

Member @james-garner-canonical james-garner-canonical yesterday I wonder if adding a type annotation to IdentityAccess.ADMIN (Literal['admin']) etc would resolve this.

Member Author @dimaqq dimaqq 1 hour ago It's not possible to add type annotations to enum members.

+1 on making the user-facing change in a separate PR, with pros and cons discussion on that separate PR.

dimaqq avatar Aug 12 '25 01:08 dimaqq