Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Upgrade mypy to 0.720, turn on unreachability warnings #25157

Merged
merged 2 commits into from
Jul 16, 2019
Merged

Upgrade mypy to 0.720, turn on unreachability warnings #25157

merged 2 commits into from
Jul 16, 2019

Conversation

scop
Copy link
Member

@scop scop commented Jul 15, 2019

Description:

http://mypy-lang.blogspot.com/2019/07/mypy-0720-released.html

Checklist:

  • The code change is tested and works locally.
  • Local tests pass with tox. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.
  • I have followed the development checklist

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. Update and include derived files by running python3 -m script.hassfest.
  • New or updated dependencies have been added to requirements_all.txt by running python3 -m script.gen_requirements_all.
  • Untested files have been added to .coveragerc.

If the code does not interact with devices:

  • Tests have been added to verify that the new code works.

@scop scop requested a review from a team as a code owner July 15, 2019 06:20
pvizeli
pvizeli reviewed Jul 15, 2019
View reviewed changes
homeassistant/util/__init__.py
@@ -53,7 +53,7 @@ def repr_helper(inp: Any) -> str:
return str(inp)


def convert(value: T, to_type: Callable[[T], U],
def convert(value: Optional[T], to_type: Callable[[T], U],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is that Optional?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because it is really optional -- the code has specific checks and treatment of None values.

@pvizeli
Copy link
Member

pvizeli commented Jul 15, 2019

There are so many https://github.com/python/mypy/issues/7207 maybe we should wait until that is fix?

@scop
Copy link
Member Author

scop commented Jul 15, 2019

Despite those (not that many IMO), unreachability checks helped catch some real issues. I think it's a net positive and should be enabled already now.

@scop
Copy link
Member Author

scop commented Jul 15, 2019

Also, the mypy 7207 was directed to python/mypy#3186 which seems to be kind of a long standing one, and might take a while to fix, and the fix might not be even actually in mypy but elsewhere. So I wouldn't wait for that from that POV either.

@balloob
Copy link
Member

balloob commented Jul 16, 2019

Looks good 👍

@balloob balloob merged commit 56841da into home-assistant:dev Jul 16, 2019
KJonline pushed a commit to Rendili/home-assistant that referenced this pull request Jul 17, 2019
Merge branch 'dev' of https://github.com/home-assistant/home-assistant …
f0e4229 
…into dev

* 'dev' of https://github.com/home-assistant/home-assistant: (156 commits)
  Add HmIP-PCBS2, HmIP-PCBS-BAT to Homematic IP Cloud (home-assistant#25201)
  Simplify cache restore (home-assistant#25186)
  Only include target temp if has right support flag (home-assistant#25193)
  Fix issue home-assistant#24495 (home-assistant#25199)
  Handle somfy expired token (home-assistant#25195)
  Add login_method config option to fix login issue with RouterOS Version > 6.43 (home-assistant#25194)
  Add HMIP-FCI / HMIP-FBL / HmIP-BBL (home-assistant#25188)
  [climate] Tweak evohome migration (home-assistant#25187)
  Fix device name customization on ZHA add devices page (home-assistant#25180)
  Upgrade mypy to 0.720, turn on unreachability warnings (home-assistant#25157)
  Use MockConfigEntry (home-assistant#25190)
  Add severe weather sensor to Dark Sky (home-assistant#22701)
  Fix typo in azure-pipelines-ci.yml
  Raise not ready when no data from API is retrieved (home-assistant#25182)
  Improve Nuki lock (home-assistant#22888)
  Delete config.yml (home-assistant#25181)
  Upgrade luftdaten to 0.6.2 (home-assistant#25177)
  Upgrade Mastodon.py to 1.4.5 (home-assistant#25176)
  Upgrade ruamel.yaml to 0.15.99 (home-assistant#25175)
  Upgrade discord.py to 1.2.3 (home-assistant#25174)
  ...
KJonline pushed a commit to Rendili/home-assistant that referenced this pull request Jul 17, 2019
Merge branch 'dev' of https://github.com/home-assistant/home-assistant …
65ecfc0 
…into hive_water_heater

* 'dev' of https://github.com/home-assistant/home-assistant: (49 commits)
  Pin Docker to Debain Stretch (home-assistant#25206)
  Updated frontend to 20190717.0
  Add HmIP-PCBS2, HmIP-PCBS-BAT to Homematic IP Cloud (home-assistant#25201)
  Simplify cache restore (home-assistant#25186)
  Only include target temp if has right support flag (home-assistant#25193)
  Fix issue home-assistant#24495 (home-assistant#25199)
  Handle somfy expired token (home-assistant#25195)
  Add login_method config option to fix login issue with RouterOS Version > 6.43 (home-assistant#25194)
  Add HMIP-FCI / HMIP-FBL / HmIP-BBL (home-assistant#25188)
  [climate] Tweak evohome migration (home-assistant#25187)
  Fix device name customization on ZHA add devices page (home-assistant#25180)
  Upgrade mypy to 0.720, turn on unreachability warnings (home-assistant#25157)
  Use MockConfigEntry (home-assistant#25190)
  Add severe weather sensor to Dark Sky (home-assistant#22701)
  Fix typo in azure-pipelines-ci.yml
  Raise not ready when no data from API is retrieved (home-assistant#25182)
  Improve Nuki lock (home-assistant#22888)
  Delete config.yml (home-assistant#25181)
  Upgrade luftdaten to 0.6.2 (home-assistant#25177)
  Upgrade Mastodon.py to 1.4.5 (home-assistant#25176)
  ...

# Conflicts:
#	homeassistant/components/hive/water_heater.py
@lock lock bot locked and limited conversation to collaborators Jul 17, 2019
@scop scop deleted the upgrade-mypy branch July 19, 2019 14:23
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@pvizeli pvizeli pvizeli left review comments

@balloob balloob balloob approved these changes

Assignees
No one assigned
Labels
cla-signed core
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@scop @pvizeli @balloob @homeassistant