Skip to content

Distribute type information in the package using PEP561 #47796

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

Closed
wants to merge 1 commit into from

Conversation

KapJI
Copy link
Member

@KapJI KapJI commented Mar 12, 2021

Proposed change

PEP 561 tells how packages can distribute information about their types.
This is useful for developers of custom components who can type check usages of homeassistant package in their code.
Also this should be useful to developers of AppDaemon libraries.

Home Assistant Core is already enforcing strict typing so I don't see reasons not to do this.

Basically this required only putting empty py.typed file in the package root and ship it with the package. setup.py already has include_package_data=True so no changes are required there.

Type of change

  • Dependency upgrade
  • Bugfix (non-breaking change which fixes an issue)
  • New integration (thank you!)
  • New feature (which adds functionality to an existing integration)
  • Breaking change (fix/feature causing existing functionality to break)
  • Code quality improvements to existing code or addition of tests

Additional information

I tested it by installing homeassistant in virtualenv with setup.py install and running mypy to type check the custom component.

Checklist

  • The code change is tested and works locally.
  • Local tests pass. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.
  • I have followed the development checklist
  • The code has been formatted using Black (black --fast homeassistant tests)
  • Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

If the code communicates with devices, web services, or third-party tools:

  • The manifest file has all fields filled out correctly.
    Updated and included derived files by running: python3 -m script.hassfest.
  • New or updated dependencies have been added to requirements_all.txt.
    Updated by running python3 -m script.gen_requirements_all.
  • Untested files have been added to .coveragerc.

The integration reached or maintains the following Integration Quality Scale:

  • No score or internal
  • 🥈 Silver
  • 🥇 Gold
  • 🏆 Platinum

To help with the load of incoming pull requests:

@KapJI
Copy link
Member Author

KapJI commented Mar 12, 2021

@balloob what do you think? 🙂

@frenck
Copy link
Member

frenck commented Mar 12, 2021

Duplicate of #28866, which has been closed.

See: https://www.python.org/dev/peps/pep-0561/#packaging-type-information

This marker applies recursively: if a top-level package includes it, all its sub-packages MUST support type checking as well.

We don't adhere to that and thus should not carry this marker.

As a matter of fact, the largest part of the code base is excluded from type checking even.

PS: Please don't ping/mention people for review.

@KapJI
Copy link
Member Author

KapJI commented Mar 12, 2021

I see, thank you!

As far I can see only components are untyped (most of them). So as far as developers don't import code from other components, it should be safe.

Have it been discussed to split components from core itself to a separate package or even a separate repo? Huge change but it makes a lot of sense to me. Should make core much easier to maintain.

@MartinHjelmare
Copy link
Member

Thanks for your contribution. I'll close here now.

@KapJI KapJI deleted the py.typed branch March 12, 2021 14:52
@github-actions github-actions bot locked and limited conversation to collaborators Mar 13, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants