Use Mypy

[wsanchez]

No description provided.

[codecov-io]

Codecov Report

Merging #80 into master will increase coverage by 0.3%.
The diff coverage is 100%.

@@            Coverage Diff            @@
##           master      #80     +/-   ##
=========================================
+ Coverage   98.18%   98.48%   +0.3%     
=========================================
  Files           8       10      +2     
  Lines        1487     1521     +34     
  Branches      180      177      -3     
=========================================
+ Hits         1460     1498     +38     
+ Misses         13       12      -1     
+ Partials       14       11      -3

Impacted Files	Coverage Δ
src/hyperlink/test/test_parse.py	100% <ø> (ø)	⬆️
src/hyperlink/test/test_decoded_url.py	100% <ø> (ø)	⬆️
src/hyperlink/test/test_socket.py	100% <100%> (ø)
src/hyperlink/test/test_scheme_registration.py	100% <100%> (+4.44%)	⬆️
src/hyperlink/test/test_common.py	100% <100%> (ø)	⬆️
src/hyperlink/_url.py	96.8% <100%> (+0.19%)	⬆️
src/hyperlink/_socket.py	100% <100%> (ø)
src/hyperlink/test/common.py	100% <100%> (ø)	⬆️
src/hyperlink/test/test_url.py	99.81% <100%> (ø)	⬆️
... and 1 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6fa88f8...a6e027e. Read the comment docs.

[wsanchez]

I'll stop here and await review before continuing.

_url.py has more to go, and some of that is also low-hanging fruit, but I see some cases where we may want to refactor a bit, so I'll try to break that up into parts.

@mahmoud Again, codecov/patch is not passing here due to touching already-uncovered lines, which I think are best addressed separately.

[wsanchez]

@glyph: FYI, I think this is up your alley.

[glyph]

Indeed it is! First question: what about CI? :)

[wsanchez]

@glyph I'd like a pass on codecov/patch.

The untested Windows code untested before (it just moved), and I think fixing that is orthogonal to this work.

Same with the lambdas in the tests… but maybe those are trivially fixed. One sec… yeah. Done.

[mahmoud]

Seeing these windows troubles... a while back I had a branch or a gist or something that did the ip checking in regex. But the problem was that operating systems differed on what edge cases they thought were valid, and that might be expected behavior. The ol' 12 standards is better than 13, and so I left it. But if Windows is enough of a headache, I can dig it up...

[wsanchez]

@mahmoud No, I think the current solution is fine. I'm just not in the give-a-crap-about Windows camp, and I think that a legit testing feature shouldn't block on an incomplete other testing feature.

That said, I think I figured out a reasonable test for inet_pton that covers the code in question, if not actually testing the result for correctness. That achieves the desired coverage, but highlights the fact the 100% coverage doesn't mean 100% complete testing… but it's better than the status quo.

[wsanchez]

OK well I don't have a Windows box but I'm finding it surprising that the Windows tests are showing are covered but that the target code is not…

In particular, I have a new test passing in socket.AF_INET6 for address_family but the targeted code isn't being hit.

Any ideas?

[wsanchez]

Oh whoops

[wsanchez]

Committed from my phone. Whoah. Go GitHub.

[wsanchez]

OK it appears that from socket import inet_pton succeeds in Python 3 on Windows, so this is only used in 2.7 on Windows. The text coverage for _socket.py is:

src\hyperlink\test\test_socket.py      29      0      4      1    97%   8->exit
204

[wsanchez]

OK I think the issue is that WSAStringToAddressA is raising if address_family is invalid, so the only way to test the current code is to send in a valid address family that we don't handle later, which would probably be a bug…

So I've re-arranged the code to do our handling of address_family before it is passed to WSAStringToAddressA, which affords us a chance to cover the code path without assuming that we are handling all possible valid values.

[wsanchez]

Well OK that did it. All done.

[glyph]

Looks great let's land it.

How do we get from here to shipping stubs?

[mahmoud]

LGTM. Just verifying, this seems to mostly be adding types to tests and some utilities. The core URL types still need typing added in future. Am I understanding that correctly?

[wsanchez]

@mahmoud: I started adding some type info to _url.py but it's not complete. I got worried that this PR was getting big so I wanted to pause here in case there were concerns with the progress so far. (Instead got a bonus project.)

I'll start on more typing in a separate PR. You are correct; the most interesting type hints for clients are still to be done. They are also the less trivial, I think. Everything here is no-brainer-ish.

@glyph: I think we should fixing the typing before we ship the type info. I don't think we'll be shipping stubs, and just using the inline type hints. It appears fairly straightforward, but I've only glanced through those docs.

[glyph]

@glyph: I think we should fixing the typing before we ship the type info.

Oh, absolutely. Hence "LGTM" :).

I don't think we'll be shipping stubs, and just using the inline type hints. It appears fairly straightforward, but I've only glanced through those docs.

Okay my understanding is that you need to build the stubs (and ship them) from the inline type hints, if you want 3rd party libraries to be able to use them. But then I only have the vaguest idea of how this all hangs together. What documentation are you referring to, outside the PEP?

[wsanchez]

What documentation are you referring to, outside the PEP?

Let's take this to #81