Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Provide type hints #243

Open
Kixunil opened this issue Aug 28, 2020 · 8 comments
Open

Provide type hints #243

Kixunil opened this issue Aug 28, 2020 · 8 comments

Comments

@Kixunil
Copy link

Kixunil commented Aug 28, 2020

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

@mcelrath
Copy link
Collaborator

Agreed. A pull request would be welcome ;-)

@ysangkok
Copy link
Contributor

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
Copy link
Contributor

One could rebase Simplexum@b275373

@dgpv
Copy link
Contributor

dgpv commented Oct 13, 2020

One could rebase Simplexum/python-bitcointx@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

@Kixunil
Copy link
Author

Kixunil commented Oct 13, 2020

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.

@dgpv
Copy link
Contributor

dgpv commented Oct 13, 2020

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
Copy link
Contributor

dgpv commented Oct 13, 2020

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.

@Kixunil
Copy link
Author

Kixunil commented Oct 13, 2020

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants