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 CI Configuration #25844

Merged
merged 17 commits into from
Mar 26, 2019
Merged

MyPy CI Configuration #25844

merged 17 commits into from
Mar 26, 2019

Conversation

WillAyd
Copy link
Member

@WillAyd WillAyd commented Mar 23, 2019

I think this is the minimal amount of work required to get MyPy configured for CI. Note that the list of files is the only documented way to whitelist things that I could find (see also python/mypy#1476)

I think we want to add this to the Checks_and_doc job though I couldn't see where that was actually configured (maybe an access thing on Azure?) so can do as a quick follow up

@jreback @gwrome

@WillAyd WillAyd added the Typing type annotations, mypy/pyright type checking label Mar 23, 2019
@pep8speaks
Copy link

pep8speaks commented Mar 23, 2019

Hello @WillAyd! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2019-03-26 13:55:38 UTC

@codecov
Copy link

codecov bot commented Mar 23, 2019

Codecov Report

Merging #25844 into master will decrease coverage by 49.53%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff             @@
##           master   #25844       +/-   ##
===========================================
- Coverage    91.3%   41.77%   -49.54%     
===========================================
  Files         173      173               
  Lines       53004    53004               
===========================================
- Hits        48397    22142    -26255     
- Misses       4607    30862    +26255
Flag Coverage Δ
#multiple ?
#single 41.77% <ø> (ø) ⬆️
Impacted Files Coverage Δ
pandas/core/internals/blocks.py 51.95% <ø> (-42.14%) ⬇️
pandas/io/formats/latex.py 0% <0%> (-100%) ⬇️
pandas/io/sas/sas_constants.py 0% <0%> (-100%) ⬇️
pandas/tseries/plotting.py 0% <0%> (-100%) ⬇️
pandas/tseries/converter.py 0% <0%> (-100%) ⬇️
pandas/io/formats/html.py 0% <0%> (-99.36%) ⬇️
pandas/core/groupby/categorical.py 0% <0%> (-95.46%) ⬇️
pandas/io/sas/sas7bdat.py 0% <0%> (-91.17%) ⬇️
pandas/io/sas/sas_xport.py 0% <0%> (-90.15%) ⬇️
pandas/core/tools/numeric.py 10.44% <0%> (-89.56%) ⬇️
... and 130 more

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 9e27f12...60d12bc. Read the comment docs.

@codecov
Copy link

codecov bot commented Mar 23, 2019

Codecov Report

Merging #25844 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master   #25844   +/-   ##
=======================================
  Coverage   91.48%   91.48%           
=======================================
  Files         175      175           
  Lines       52885    52885           
=======================================
  Hits        48381    48381           
  Misses       4504     4504
Flag Coverage Δ
#multiple 90.04% <ø> (ø) ⬆️
#single 41.82% <ø> (ø) ⬆️
Impacted Files Coverage Δ
pandas/core/internals/blocks.py 94.08% <ø> (ø) ⬆️

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 b878f5b...eeb266e. Read the comment docs.

@jreback
Copy link
Contributor

jreback commented Mar 23, 2019

ci/code_checks.sh

@WillAyd
Copy link
Member Author

WillAyd commented Mar 23, 2019

Yep thanks - was in the process of updating that and actually just pushed. Wasn't sure where Azure created the environment to run that script but may not matter for this

axis=0, # type: libinternals.BlockPlacement
fill_value=None): # type: Any
# type: (...) -> List[ExtensionBlock]
periods: int,
Copy link
Member Author

Choose a reason for hiding this comment

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

This was kind of annoying to fix some syntax issues and make this work with LINT so I just converted to new syntax to avoid hassle

@WillAyd
Copy link
Member Author

WillAyd commented Mar 23, 2019

Do I just need to add mypy to setup.cfg to get that installed?

@jreback
Copy link
Contributor

jreback commented Mar 23, 2019

how does this not fail the CI?

@WillAyd
Copy link
Member Author

WillAyd commented Mar 23, 2019 via email

@jreback
Copy link
Contributor

jreback commented Mar 23, 2019

you need to add to azure-pipelines.yml

ci/code_checks.sh Outdated Show resolved Hide resolved
@WillAyd
Copy link
Member Author

WillAyd commented Mar 23, 2019

Ah OK thanks - kept looking in the ci folder so no wonder I couldn't find it...just pushed some updates which hopefully should fail typing.

@WillAyd
Copy link
Member Author

WillAyd commented Mar 23, 2019

OK mypy failures showing up in logs as expected, so looks like the CI side is configured:

https://dev.azure.com/pandas-dev/pandas/_build/results?buildId=9736&view=logs&jobId=10b0523a-38de-5c7a-02e7-c2259d583b33&taskId=afefc423-d7a4-5e71-9b36-a62b9d093e90&lineStart=15&lineEnd=16&colStart=1&colEnd=1

Going to push now looking only at a whitelist of modules should go green

mypy.ini Show resolved Hide resolved
@@ -0,0 +1,4 @@
pandas/core/arrays/base.py
Copy link
Contributor

Choose a reason for hiding this comment

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

can you add some comments here to indicate what this file is. Also can you add a small new doc section on typing (just a starter is good, can expand over time). on how to type / how to update this file etc.

@@ -0,0 +1,4 @@
pandas/core/arrays/base.py
pandas/core/arrays/datetimes.py
pandas/core/common.py
Copy link
Contributor

Choose a reason for hiding this comment

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

really wish this were a blacklist file :>

@jreback jreback added this to the 0.25.0 milestone Mar 24, 2019
@jreback
Copy link
Contributor

jreback commented Mar 24, 2019

also its worth a note in the whatsnew, maybe in a development section? eventually we will want to expose pandas.typing but not yet.

@WillAyd
Copy link
Member Author

WillAyd commented Mar 26, 2019

OK latest commit goes with an alternate strategy to blacklist individual modules. It's more verbose but is self-contained within the configuration file and is probably easier to manage and orient tasks for in a project. Let me know what you think

Copy link
Contributor

@jreback jreback left a comment

Choose a reason for hiding this comment

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

looks fine. ping on green.

@WillAyd
Copy link
Member Author

WillAyd commented Mar 26, 2019

@jreback all green

@jreback jreback merged commit 923ac2b into pandas-dev:master Mar 26, 2019
@jreback
Copy link
Contributor

jreback commented Mar 26, 2019

thanks!

@WillAyd WillAyd added this to In progress in Type Hints via automation Mar 26, 2019
@WillAyd WillAyd moved this from In progress to Done in Type Hints Mar 26, 2019
@WillAyd WillAyd deleted the mypy-ci branch January 16, 2020 00:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Typing type annotations, mypy/pyright type checking
Projects
No open projects
Type Hints
  
Done
Development

Successfully merging this pull request may close these issues.

adding static type checking with mypy
3 participants