mypy error: Variable "watchdog.observers.Observer" is not valid as a type

[JohnStrunk]

Since the release of 3.0.0, I'm getting errors from mypy when I use Observer as a type in my function definitions:

from watchdog.observers import Observer

def myfn(obs: Observer) -> None:
    pass

myobs = Observer()
myfn(myobs)

> mypy zz.py
zz.py:3: error: Variable "watchdog.observers.Observer" is not valid as a type  [valid-type]
zz.py:3: note: See https://mypy.readthedocs.io/en/stable/common_issues.html#variables-vs-type-aliases
Found 1 error in 1 file (checked 1 source file)

> python --version
Python 3.11.0

Platform: Windows

[BoboTiG]

Thanks @JohnStrunk for such a clean report :)

@altendky would it be possible to have a look 🙏🏻 ?

[altendky]

Yep, thanks for the clear report. I see how what I added fixed one issue but not another and it needs to be fixed. I think this may involve some other changes in approach elsewhere. Let's look at this a bit and start with considering just the Linux case as an example.

https://github.com/gorakhargosh/watchdog/blob/a4f28a1bdebae67c67d70082de38256e80a0f253/src/watchdog/observers/init.py#L65-L69

So on Linux I think we want the hint to applications to be Union[Type[InotifyObserver], Type[PollingObserver]] since the intent, as I read it, is to have the library prefer inotify when possible but for the application to be able to work properly with either in case inotify is not available. We can't presently hint that because we can't even get the InotifyObserver type unless inotify is actually available. I guess at hint time mypy wouldn't care about the exception, but it would be good to separate the type availability from the ability to use it.

https://github.com/gorakhargosh/watchdog/blob/a4f28a1bdebae67c67d70082de38256e80a0f253/src/watchdog/observers/inotify_c.py#L31-L36

So we will want to adjust this so that the mechanism for checking availability doesn't block access to the class. Then we can import both possibilities and provide a hint covering them both before identify which to actually use and assigning it to watchdog.observers.Observer.

I'll try to make some time for this but I disappeared due to some more urgent activities at work. I do still plan to come back and work on both tests here and more hinting. I expect hinting more ourselves would uncover issues like this in CI so that users don't have to hit them after we release. Sorry for the hassle @JohnStrunk.

Side note, we should add mypy checking for both BSD and 'unknown' systems since we have code trying to handle them. That can be done by passing a --platform to mypy.

I just tried to do a bit of a hacky workaround but ran into some issues. I'll let you know if I come up with something. Let me know if you do. :]

[altendky]

Maybe this is a relevant minimal example of the issue.

https://mypy-play.net/?mypy=1.1.1&python=3.11&gist=08674707b9b4b878a182dc85a145c91f

from typing import Type, Union


class Base:
    ...

class M(Base):
    ...

class N(Base):
    ...

# a runtime condition that mypy should not resolve
runtime_condition: bool

C: Union[Type[M], Type[N]]
D: Type[Base]

if runtime_condition:
    C = M
    D = M
else:
    C = N
    D = N

ia: C
ib: D

if runtime_condition:
    E = M
else:
    E = N

ic: E

main.py:26: error: Variable "__main__.C" is not valid as a type  [valid-type]
main.py:26: note: See https://mypy.readthedocs.io/en/stable/common_issues.html#variables-vs-type-aliases
main.py:27: error: Variable "__main__.D" is not valid as a type  [valid-type]
main.py:27: note: See https://mypy.readthedocs.io/en/stable/common_issues.html#variables-vs-type-aliases
main.py:32: error: Cannot assign multiple types to name "E" without an explicit "Type[...]" annotation  [misc]
main.py:32: error: Incompatible types in assignment (expression has type "Type[N]", variable has type "Type[M]")  [assignment]
Found 4 errors in 1 file (checked 1 source file)

[nhairs]

I've also encountered this issue.

Doing some digging it looks like a lot of the problem is stemming from the fact that Observer is not actually a type but a type alias. I'll be honest, reading the docs on the matter does my head in a little bit, so maybe you'll have better luck.

I also feel like trying to narrow the type of Observer when type checking is a mistake as the type checker can't actually guarantee the actual type of Observer at runtime (proof being that you can coerce the platform in MyPy). If you can indeed guarantee the platform at runtime you shouldn't be importing Observer but instead the concrete implementation that you will be using.

As such I believe the correct type of Observer should actually be watchdog.oberservers.api.BaseObserver as this is the only typing information you can guarantee at runtime. (While I'm not sure I fully understand it, it feels like attempting to narrow the type is breaking Liskov's Substitution Principal)

I was also wondering if Protocols might help, but it looks like that is what you are already attempting which might be the cause of the problem? (watchdog.observers.__init__ ref, watchdog.observers.api ref)

Alternatively, while the "black-magic" of replacing Observer with the best implementation based on platform is super cool. Perhaps it's actually too much black-magic and instead we should be using a function that returns the "best" implementation. (Explicit is better than implicit)

def get_best_observer_class() -> watchdog.observers.api.BaseObserver:
    "Get the best observer class available for your platform"
    if sys.platform.startswith("linux"):
        ...

Work around for libraries depending on Watchdog

The work-around for code calling watchdog is to replace watchdog.observers.Observer with watchdog.obersvers.api.BaseObserver in your type hinting.

[BoboTiG]

The simplest solution, as @nhairs pointed, is to use BaseObserver for the type:

@@ -1,7 +1,7 @@
-from watchdog.observers import Observer
+from watchdog.observers import BaseObserver, Observer
 
-def myfn(obs: Observer) -> None:
+def myfn(obs: BaseObserver) -> None:
     pass
 
 myobs = Observer()
 myfn(myobs)

I would close the issue since this is a clean one. OK for you?

[JohnStrunk]

@BoboTiG, thanks for distilling that solution.

I'm fine w/ closing the issue. If I run into problems making the change, I'll re-open for further discussion.