Codecov Report

Attention: Patch coverage is 85.36585% with 12 lines in your changes are missing coverage. Please review.

Project coverage is 71.00%. Comparing base (0a4f40e) to head (c4cd0ce).

@@            Coverage Diff             @@
##             main     #610      +/-   ##
==========================================
- Coverage   71.28%   71.00%   -0.28%     
==========================================
  Files          13       13              
  Lines        1069     1083      +14     
==========================================
+ Hits          762      769       +7     
- Misses        307      314       +7     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Essentially just adds a few more types to the public interface

There is no public interface, and the README says as much. See #262 for a discussion on that.

Fair enough, bad choice of words on my part. I used the word “public” because I’ve recently seen a few uses of bdist_wheel subclasses in setup.py files. Seems like a known way of making sure wheels are marked as abi3.

I figured improving the typing overall might make such usage a bit easier while not affecting anything the wheel package actually does.

Even if all this provides is a few quality of life improvements for typing users who have wheel directly in their code, I hope that's still in the library's interests to have (even without an established public API). Anything I can do to fix this or make it more appealing, e.g. removing the pyright configuration?

Just following up, @agronholm:

Would you consider this PR, even if it doesn't change functionality at all? Might be unnecessary, but to make my case again:

It's more like documentation or typing aid for typing users that use wheel at runtime (even without a public interface/API). As previously mentioned, it doesn't seem uncommon to use wheel in setup.py by subclassing bdist_wheel for modification of distribution metadata, e.g. setting py_limited_api based on Python version and platform. Making it a tiny bit easier for some users to do that seems like a net benefit, but I'm not sure if you'd prefer fewer annotations to more. That's perfectly valid, but there's a mix of typed and untyped code in the package already, and I'd perhaps consider this PR as an improvement of that ratio?

Hope my reasons makes sense, and I'm more than willing to take feedback if I'm wrong.

Just following up, @agronholm:

Would you consider this PR, even if it doesn't change functionality at all? Might be unnecessary, but to make my case again:

It's more like documentation or typing aid for typing users that use wheel at runtime (even without a public interface/API). As previously mentioned, it doesn't seem uncommon to use wheel in setup.pyby subclassingbdist_wheelfor modification of distribution metadata, e.g. settingpy_limited_api` based on Python version and platform. Making it a tiny bit easier for some users to do that seems like a net benefit, but I'm not sure if you'd prefer fewer annotations to more. That's perfectly valid, but there's a mix of typed and untyped code in the package already, and I'd perhaps consider this PR as an improvement of that ratio?

Hope my reasons makes sense, and I'm more than willing to take feedback if I'm wrong.

Well, so long as we only use these annotations to sort of improve the internal documentation, and don't advertise them by adding a py.typed marker, I don't think I have a problem with it.

More than fair, thank you. If there's anything else I can do, don't hesitate to let me know.

Thanks.

No problem! Thanks for considering it.