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.

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

[mypy] Adding mypy type checking #7053

Merged
merged 1 commit into from
Mar 25, 2019

Conversation

john-bodley
Copy link
Member

@john-bodley john-bodley commented Mar 19, 2019

This PR adds mypy (courtesy of flake8) for checking Python type hints. Additionally it updates the CONTRIBUTING.md to strongly encourage new Python functions to use type hints.

Note I corrected some mypy issues but ignored others as I don't have full context into the code. The get_since_until method seems to be especially problematic partially because since, until, and relative_end are being re-typed in the function (which is probably non-ideal).

to: @betodealmeida @michellethomas @mistercrunch @xtinec

@john-bodley john-bodley force-pushed the john-bodley--mypy branch 9 times, most recently from c24d741 to c8cd3af Compare March 19, 2019 05:11
@codecov-io
Copy link

codecov-io commented Mar 19, 2019

Codecov Report

Merging #7053 into master will not change coverage.
The diff coverage is 85.71%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #7053   +/-   ##
=======================================
  Coverage   64.46%   64.46%           
=======================================
  Files         421      421           
  Lines       20537    20537           
  Branches     2247     2247           
=======================================
  Hits        13240    13240           
  Misses       7170     7170           
  Partials      127      127
Impacted Files Coverage Δ
superset/common/query_context.py 22.85% <ø> (ø) ⬆️
superset/common/query_object.py 26.31% <0%> (ø) ⬆️
superset/utils/core.py 88.22% <100%> (ø) ⬆️

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 c7ffdd6...4b7ac67. Read the comment docs.

@@ -462,6 +462,35 @@ Note that the test environment uses a temporary directory for defining the
SQLite databases which will be cleared each time before the group of test
commands are invoked.

#### Typing

To ensure clarity, consistency, all readability, _all_ new functions should use
Copy link
Member

Choose a reason for hiding this comment

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

Personally I'm happy with this. Can we kick off a [DISCUSS] thread on the mailing list though, making sure we have consensus?

@mistercrunch
Copy link
Member

LGTM !
Also worth checking this out: https://github.com/Instagram/MonkeyType

@mistercrunch
Copy link
Member

I'm merging this as I'd like to play around with mypy and add some type annotations to some utils/ functions

@mistercrunch mistercrunch merged commit 80d6f5a into apache:master Mar 25, 2019
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 0.34.0 labels Feb 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 0.34.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants