-
-
Notifications
You must be signed in to change notification settings - Fork 699
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
mypy error: Variable "watchdog.observers.Observer" is not valid as a type #982
Comments
Thanks @JohnStrunk for such a clean report :) @altendky would it be possible to have a look 🙏🏻 ? |
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. watchdog/src/watchdog/observers/__init__.py Lines 65 to 69 in a4f28a1
So on Linux I think we want the hint to applications to be watchdog/src/watchdog/observers/inotify_c.py Lines 31 to 36 in a4f28a1
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 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 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. |
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
|
I've also encountered this issue. Doing some digging it looks like a lot of the problem is stemming from the fact that I also feel like trying to narrow the type of As such I believe the correct type of I was also wondering if Alternatively, while the "black-magic" of replacing 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 WatchdogThe work-around for code calling |
Also updates Observer type hints based on gorakhargosh/watchdog#982
#2031 Remove EDDB Code # Added - Imported temporary type hints per gorakhargosh/watchdog#982 # Changed - A few defaults for URL settings - Updated Dependencies to latest working versions - Minor documentation updates # Removed - Removes EDDB Plugin and References
The simplest solution, as @nhairs pointed, is to use @@ -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? |
@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. |
There's probably 2 things that we can do to help reduce the number of people that hit this issue. The first would be to add documentation, probably to the Quick Start guide explaining that for typing purposes you should use The second would be to add proper type annotations to edit: I see that we've added the edit2: I see that the published docs are out of date. |
* [docs] Add typing info to quick start ref: #982 * Add to authors (including fixing ordering) * Fix base module --> api * Update docs/source/quickstart.rst --------- Co-authored-by: Mickaël Schoentgen <[email protected]>
Since the release of 3.0.0, I'm getting errors from mypy when I use
Observer
as a type in my function definitions:Platform: Windows
The text was updated successfully, but these errors were encountered: