search

gorakhargosh / watchdog

Python library and shell utilities to monitor filesystem events.
http://packages.python.org/watchdog/
Apache License 2.0
6.53k stars 695 forks source link

feat!: Enable Mypy `disallow_untyped_defs` rule + fixes #1060

Closed BoboTiG closed 1 month ago

danielhollas commented 1 month ago

Thanks so much for adding these types! :heart:

I've just run into them missing while adding types in our package at https://github.com/aiidalab/aiidalab/pull/453. If you plan to release this in near-ish future we can wait instead of adding a bunch of type-ignore.

BoboTiG commented 1 month ago

Can you try the master branch to see how all those chages work for you? I made significant breaking changes ^^

danielhollas commented 1 month ago

Yep, happy to test tomorrow, thanks for taking a look!

danielhollas commented 1 month ago

I did a very quick check, and I am currently getting the following mypy error when trying to create the Observer() object

aiidalab/app.py:624: error: Missing positional argument "emitter_class" in call to "BaseObserver"  [call-arg]
Found 3 errors in 2 files (checked 1 source file)

Was that part of the breaking changes or is mypy just confused somehow? The other types seem to work fine.

BoboTiG commented 1 month ago

It seems incorrect on the watchdog end, at first sight. I'll check.

BoboTiG commented 1 month ago

It's fixed with 9af32e07968166d073a39d8581b2ac8e422c9116, can you double check please?

danielhollas commented 1 month ago

Yep, that fixed it, thanks!

I made significant breaking changes ^^

Can you elaborate on this a bit more? Are these all documented in the changelog?

Looking at it it doesn't feel like we should be impacted since we only really use the Observer class. We might need to handle the new IN_CLOSE_NOWRITE event. Our unit test suite is passing, but we admittedly do not have a high coverage of this functionality. I'll try to do a more end-to-end testing later.

BoboTiG commented 1 month ago

Can you elaborate on this a bit more? Are these all documented in the changelog?

Yes. The biggest one is "[core] Enforced usage of proper keyword-arguments", but it should be either easy to fix because Mypy will spot them, or do not impact you.