Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Story sub-objects not being type-checked by mypy??? #233

Open
philbudne opened this issue Feb 5, 2024 · 11 comments
Open

Story sub-objects not being type-checked by mypy??? #233

philbudne opened this issue Feb 5, 2024 · 11 comments
Labels
bug Something isn't working
Milestone

Comments

@philbudne
Copy link
Contributor

from indexer.story import BaseStory

def foo(s: BaseStory) -> None:
    with s.rss_entry() as rss:
        # unpleasant discovery #1: mypy displays:
        # zzz.py:7: note: Revealed type is "Any"
        reveal_type(rss)


def bar(s: BaseStory) -> None:
    rss = s.rss_entry()
    with rss:
        # mypy displays:
        # zzz.py:15: note: Revealed type is "indexer.story.RSSEntry"
        reveal_type(rss)

        # BUT (unpleasant discovery #2)
        # mypy doesn't complain about bogus fields
        # (but will type check actual fields)
        rss.zzz = 1
@philbudne
Copy link
Contributor Author

NOTE! we use "pickle" to transmit Stories in queue messages. Any change to internal format MUST preserve the ability to process already queued messages (for example, ones trapped in quarantine queues), or at least any deployment of a breaking change needs to be done carefully (emptying all queues first)!!!!!

@rahulbot rahulbot added the bug Something isn't working label Feb 14, 2024
@rahulbot rahulbot added this to the Production Beta 4 milestone Feb 14, 2024
@rahulbot rahulbot modified the milestones: Production Beta 4, long-term Mar 13, 2024
@philbudne
Copy link
Contributor Author

Appears to be a mypy bug, not related to dataclasses either:

class Foo:
    a: int

    def __enter__(self):
        return self

    def __exit__(self, *args):
        pass

f1 = Foo()
with f1:
    reveal_type(f1)
    f1.a = 1
    f1.b = 2

with Foo() as f2:
    reveal_type(f2)
    f2.a = 1
    f2.b = 2

def f() -> Foo:
    return Foo()

with f() as f3:
    reveal_type(f3)
    f3.a = 1
    f3.b = 2
(venv) pbudne@ramos:~/typing$ mypy with.py 
with.py:12: note: Revealed type is "with.Foo"
with.py:14: error: "Foo" has no attribute "b"  [attr-defined]
with.py:17: note: Revealed type is "Any"
with.py:25: note: Revealed type is "Any"
Found 1 error in 1 file (checked 1 source file)

@philbudne
Copy link
Contributor Author

@kilemensi since I investigated due to your review comment:

I asked on a python typing chat (before opening a mypy issue) and got the answer that the __enter__ method needs to be properly typed (default to Any if no annotation), and right now StoryData.__enter__ explicitly returns Any:

     # Implementing typing on return:self is really finicky, just doing Any for now
    def __enter__(self) -> Any:
        self.frozen = False
        return self

I'm not quite sure how to fix this short of having each StoryData subclass have an explicit __enter__ declaration, and none in StoryData itself?

@philbudne
Copy link
Contributor Author

also @pgulley since you created the Story class framework

@kilemensi
Copy link
Contributor

Can't think of any other way out either @philbudne. Being on 3.10, I think we can use TypeVar to make __enter__/__exit_ generic in the base StoryData. In 3.11, we can switch to Self.

@philbudne
Copy link
Contributor Author

Found this, but didn't try anything out: python/mypy#3839

@kilemensi
Copy link
Contributor

Looks like TypeVar is enough to solve our current conundrum... based on your code:

from typing import TypeVar

TFoo = TypeVar("TFoo", bound="Foo")

class Foo:
    a: int

    def __enter__(self: TFoo) -> TFoo:
        return self

    def __exit__(self: TFoo, *args) -> None:
        pass

class Bar(Foo):
    pass

class Baz(Foo):
    b: int

# Foo
f1 = Foo()
with f1:
    reveal_type(f1)
    f1.a = 1
    f1.b = 2

with Foo() as f2:
    reveal_type(f2)
    f2.a = 1
    f2.b = 2

def f() -> Foo:
    return Foo()

with f() as f3:
    reveal_type(f3)
    f3.a = 1
    f3.b = 2

# Bar
r1 = Bar()
with r1:
    reveal_type(r1)
    r1.a = 1
    r1.b = 2

with Bar() as r2:
    reveal_type(r2)
    r2.a = 1
    r2.b = 2

def r() -> Bar:
    return Bar()

with r() as r3:
    reveal_type(r3)
    r3.a = 1
    r3.b = 2

# Baz
z1 = Baz()
with z1:
    reveal_type(z1)
    z1.a = 1
    z1.b = 2

with Baz() as z2:
    reveal_type(z2)
    z2.a = 1
    z2.b = 2

def z() -> Baz:
    return Baz()

with z() as z3:
    reveal_type(z3)
    z3.a = 1
    z3.b = 2
(venv) clemence@CFA-CLEMENCE story-indexer % mypy ~/foo.py
~/foo.py:11: error: Function is missing a type annotation for one or more arguments  [no-untyped-def]
~/foo.py:24: note: Revealed type is "foo.Foo"
~/foo.py:26: error: "Foo" has no attribute "b"  [attr-defined]
~/foo.py:29: note: Revealed type is "foo.Foo"
~/foo.py:31: error: "Foo" has no attribute "b"  [attr-defined]
~/foo.py:37: note: Revealed type is "foo.Foo"
~/foo.py:39: error: "Foo" has no attribute "b"  [attr-defined]
~/foo.py:44: note: Revealed type is "foo.Bar"
~/foo.py:46: error: "Bar" has no attribute "b"  [attr-defined]
~/foo.py:49: note: Revealed type is "foo.Bar"
~/foo.py:51: error: "Bar" has no attribute "b"  [attr-defined]
~/foo.py:57: note: Revealed type is "foo.Bar"
~/foo.py:59: error: "Bar" has no attribute "b"  [attr-defined]
~/foo.py:64: note: Revealed type is "foo.Baz"
~/foo.py:69: note: Revealed type is "foo.Baz"
~/foo.py:77: note: Revealed type is "foo.Baz"
Found 7 errors in 1 file (checked 1 source file)

@philbudne
Copy link
Contributor Author

philbudne commented Mar 22, 2024 via email

@philbudne
Copy link
Contributor Author

philbudne commented Mar 22, 2024 via email

@kilemensi
Copy link
Contributor

I think (not 100% sure) this is because of the custom __setattr__ ... the question of which fields are/aren't allowed now becomes a runtime issue that mypy can't help us with.

@philbudne
Copy link
Contributor Author

philbudne commented Mar 25, 2024 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants