Skip to content

Add typing stubs #195

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

EmmaJaneBonestell
Copy link

This pull request adds some basic configuration for non-automated running of mypy, and obviously the typing stubs and py.typed marker. Only one source file was modified to replace the private _frozen_importlib(_external) import with importlib.machinery. I could see no reason for this being used, and it made typehinting the Loaders extremely verbose, difficult, and required silencing of multiple errors.

If you do not want the maintenance-burden, feel free to close this, and I will submit to typeshed instead.

The stubs were compared to API reference docs. The private code that had doc-strings that were not exported in the API reference was also checked against, but there may be some discrepancies that I missed there.

In the stubs, I added simple # documented comments to functions/classes/methods/attributes. Additionally, discrepancies like contradicting documentation such as missing or extra parameters, different parameter types, or normal attributes being called properties were commented on. You can get the more relevant comments  with something like grep -RIP '#(?!\s(?:documented$|START|END|noqa|@))'.

However, in distlib.resources, two probable Liskov Substitution Principle violations and one outright error are explicitly marked with FIXME. Besides these, it passes mypy's strict mode as well as stubtest.

These stubs are essentially complete except for the omission of compat; however, for any maintainers or for anyone else that comes across this, the following are ways these stubs may be improved or revised:

  1. Fix the commented documentation discrepancies and then remove said comments

  2. Add ClassVar[...] to class variables that should not be useable as an instance variable.

  3. Add Final[...] to variables/attributes that should not be reassigned, redefined, or overridden. This alone is not quite the same as a constant. It does not work with try/except or with if/else statements unless it is written as a ternary. Final only prevents name re-binding. If immutability is desired, an ABC is required (also see 6. below). Example from docs:

Alpha: Final[str] = ['a', 'b']
Beta: Final[Sequence[str]] = ['a', 'b']
Alpha.append('c')  # this works
Beta.append('c')  # Error

4.1 Add @final decorator to:
  A. Classes that should not be inherited from.
  B. Methods that should not be overriden in a subclass.
4.2 Evaluate whether existing @property methods should be @final to prevent overriding.

  1. Add TypedDicts for distlib.metadata. In that module, I made a comment about this and not knowing all possible keys nor their acceptable types.

  2. Use collection.abc ABC's for parameters where appropriate.

  3. Protocols could be used to enforce instance attribute requirements, e.g. distlib.index's PackageIndex.upload_file's metadata parameter is documented as requiring a Metadata instance with at least Name and Version fields set. If this level of enforcement is desired, I believe Protocols can do it.

_frozen_importlib/_frozen_importlib_external
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

Successfully merging this pull request may close these issues.

1 participant