DanielSank / observed

Observer pattern in python
MIT License
33 stars 4 forks source link

Adds type annotations #23

Open ghost opened 4 years ago

ghost commented 4 years ago

Adds type annotations

Hi! This PR solves the issue #20. It still is a pre PR because I have some things to discuss:

Let's evaluate those points before approving this PR.

DanielSank commented 2 years ago

I've added typing lib;

Ok!

All tests are passing. Should I add type annotations on tests too ?

Yes, please, but let's do it in a separate pull request to keep reviews easier.

Whenever the return value is the same as the wrapped function, I added "Any" type annotation (Ex: line 116);

Ok, we should look into whether there's some way to do better, i.e. with some form of generic types.

So, I ask special care when reviewing "key" variable (Ex: line 261);

Thanks for calling this out.

I've added "ObservableBoundMethod" as type on line 570. Since it is being called, should I use "Any" ? The same goes on line 611; I've added "Any" whenever ObservableUnboundMethod could not be accessed

I do not quite understand this comment. I'll see if I can figure it out in review.

By the way, you can put comments like this in line by clicking on lines in the "files changed" tab in the pull request.