ploxiln/fab-classic

add type annotations

timsnyder opened this issue · 5 comments

@ploxiln would you be interested in a PR that adds type annotations to fab-classic? I could slog through doing it if you have interest in accepting it.

Not at this time, sorry. Maybe after python2 support is dropped (maybe later this year).

Sounds good. I'm creating them as .pyi files that I might put on PYPI as a separate types/stubs package if you don't want them in your codebase yet. Once you do get there, I think there are easy ways to apply the types to your code with tools like retype.

Also, fab-classic is in what you might call maintenance-mode (and that was intended when creating the fork) ... so I'd generally prefer to avoid large-scale changes. So, after python2 support is dropped, I'd prefer to have annotations added to functions/areas as they are touched, rather than everywhere at once. This also makes it easier to give full attention to each little bit, to make sure the approach is correct. Python type annotations can both help the typechecker tell you where something is wrong, AND tell the typechecker to just assume something is a particular type, that it might not actually be. I've seen mistakes made related to this, while people massage things to just try to get them to pass, while doing a large tree-wide overhaul.

I completely understand. I'm was just trying to avoid situations where we tell the typechecker to completely ignore the types for fab-classic and look into how rough it would be to start adding types to only the portions of the interface api I care about.

Thanks for replying to the ticket. I'll definitely circle back with you before posting anything.

trying to avoid situations where we tell the typechecker to completely ignore the types for fab-classic and look into how rough it would be to start adding types to only the portions of the interface api I care about

That makes perfect sense to me! Sorry for being a bit defensive :)