python-bitcoinlib icon indicating copy to clipboard operation
python-bitcoinlib copied to clipboard

Published 20 hours ago • verified publisher icon petertodd

Provide type hints

Open Kixunil opened this issue 5 years ago • 8 comments

I hope the benefits for possibly security-critical library are obvious.

Kixunil avatar Aug 28 '20 06:08 Kixunil

Agreed. A pull request would be welcome ;-)

mcelrath avatar Sep 23 '20 16:09 mcelrath

I have submitted PR #247 which could be seen as a preliminary thing to do before doing type hints. Full type hinting needs Python 3.6, and type hinting on just functions needs Python 3 anyway, so Python 2 support may as well be removed since it won't even parse on there if this is added in a nice way.

ysangkok avatar Oct 13 '20 01:10 ysangkok

One could rebase https://github.com/Simplexum/python-bitcointx/commit/b275373c62ed57ce617f20e4963fb47975cf46cc

ysangkok avatar Oct 13 '20 02:10 ysangkok

One could rebase Simplexum@b275373

I don't think that it would be that simple, there was significant incompatibe changes made at this stage.

But it could cover some portion of the code, and maybe that could save some code-editing time.

It would need careful review afterwards, as any automated, but not-straightforward code change

dgpv avatar Oct 13 '20 08:10 dgpv

I think Python 2 is EOL for a long time anyway, is there a good reason to support it?

I noticed that some type hints can use a separate library that makes them work for older versions. I'm not a Python expert, don't know the specifics but it seems to work at least in some cases.

Kixunil avatar Oct 13 '20 14:10 Kixunil

Python2 is declared unsupported since v0.11.0 (in release-notes for v0.10.2, "Note: this will be the last release of python-bitcoinlib with Python 2.7 compatibility."), I believe what @ysangkok meant is that the code that deals with python2/3 compatibility shall be removed, and this is the focus of #247

There's a style of type hints that can be specified as comments, like a = None # type: Optional[int], and were used before python3.6 that allowed type hints in the normal code.

Python3.6 is also the lowest supported version of python3 (https://pythoninsider.blogspot.com/2020/10/python-35-is-no-longer-supported.html), so I think that targeting it, and ditching version 3.5 and lower is sensible.

dgpv avatar Oct 13 '20 15:10 dgpv

Because bitcointx uses type hints directly expressed in source (only supported since python3.6) taking type hint code from bitcointx would transfer the python3.6 requirement of bitcointx to bitcoinlib.

dgpv avatar Oct 13 '20 15:10 dgpv

Sounds good, even Debian stable, which is quite conservative has Python 3.7.3 already, so it should be safe, I think.

Kixunil avatar Oct 13 '20 15:10 Kixunil