search

darosior / python-bip380

Bitcoin Output Script Descriptors with Miniscript extension
MIT License
21 stars 5 forks source link

Add PEP484 type hints #17

Open stickies-v opened 2 years ago

stickies-v commented 2 years ago

I find PEP484 type hints make reading and debugging significantly easier for non-trivial functions. As I went through the codebase I added type hints everywhere, initially just for my understanding but then I thought you might like to have it here too so I went for full coverage.

As there is some maintenance burden I'd understand if you prefer not to merge it, it's personal preference. It's a lot of LoC changed, but type hints do not affect program behaviour so I think this can be pretty low-touch review (a wrong type hint can always be fixed later). Happy to split this up in smaller commits too if that makes it easier.

stickies-v commented 2 years ago

Rebased to fix merge conflicts

darosior commented 2 years ago

Thanks, just to let you know: i saw this PR, i'm a bit on the fence about it. I'll think about it.

stickies-v commented 2 years ago

Yup no problem. I've just force pushed an update to directly import the used types to improve readability (at cost of naming collision, but the trade-off seems worthwhile here). So instead of typing.Optional[typing.List[bytes]], it's now just Optional[List[bytes]]