Add Type Hints - Githubissues

paulc / dnslib

A Python library to encode/decode DNS wire-format packets

https://github.com/paulc/dnslib

BSD 2-Clause "Simplified" License

295 stars 84 forks source link

Add Type Hints #56

Open nhairs opened 6 months ago

nhairs commented 6 months ago

It would be good if dnslib added and exposed typing information so that calling libraries can check they are passing the correct types.

To support inline type hints would require dropping support for python<3.5, however given the security support schedule it may be desirable to drop support up to 3.7. Checking PyPI Stats, versions <=3.6 make up less than 1% of daily downloads.

I'd be willing to help out with this.

paulc commented 6 months ago

I'd agree and to be honest it would probably be sensible to drop support for anything < Python 3.7 but to be honest dnslib is a pretty old codebase now (has been on Github for 14 years and was developed on Bitbucket before then for a few years - I think I started it in 2007) and is pretty crufty in places - in particular when I ported from Python 2 to Python 3 it added a lot quite a lot of complexity to maintain Python 2 support - so I've tried to do the minimum necessary to keep it going. It probably needs quite a significant amount of rework to clean up and align with more modern Python coding standards and I'm not sure that would be worth the effort.

I'm closing this for the moment but would be open to a PR which drops Python 2 and Python < 3.7 support as a first step.

nhairs commented 6 months ago

Thanks for that @paulc

Since I intend to provide a PR I feel like it would be better to leave this issue open to indicate that the issue has not been resolved nor has it been rejected.

paulc commented 6 months ago

No problem - I've reopened.

Thanks, Paul

nhairs commented 6 months ago

Thanks @paulc

How would you like me to handle PRs for the py37 migration and adding of type hints?

While I'm here I also intend to:

refactor / cleanup / modernise the library
- I notice there's a few places where you have written "we do this because of compatibility", if there's anything you think I should take a look at let me know.
add standard development practices (formatter, linter, etc)
add ability to generate full docs (similar to what I've done for NServer)

I can do things incrementally, but in doing so I'd want to not release until I'm done with the work / it would require more attention over the next few weeks from you to keep things moving.

EDIT: so as I get further through adding the types I'm finding more <py37 code that should be removed. It's going to be pretty annoying to remove/refactor that code and do the typing in two different steps.