CLN: [WIP] trial pull-request to make sure everything is in order before proceeding (GH14468) #15866

brmc · 2017-04-02T05:49:52Z

The most uncertain aspect is how to annotate "array-like." numpy doesn't even have a clear definition or recommendation. see the commit message for 60783c7 and numpy/numpy/issues/7370 for more details.

For now pandas.types.hinting are essentially just place holders for complex types that need to be defined.

Any feed back is appreciated

jreback · 2017-04-02T14:26:03Z

pandas/core/base.py:4:1: F401 'typing.Union' imported but unused
pandas/core/base.py:4:1: F401 'typing.Tuple' imported but unused
pandas/core/base.py:4:1: F401 'typing.Callable' imported but unused
pandas/core/base.py:4:1: F401 'typing.Optional' imported but unused
pandas/core/base.py:4:1: F401 'typing.Any' imported but unused
pandas/core/base.py:10:1: F401 'pandas.types.hinting.ArrayLike' imported but unused
pandas/core/base.py:10:1: F401 'pandas.types.hinting.Buffer' imported but unused
pandas/core/base.py:10:1: F401 'pandas.types.hinting.PythonScalar' imported but unused
pandas/core/base.py:10:1: F401 'pandas.types.hinting.Scalar' imported but unused
pandas/types/hinting.py:10:59: W292 no newline at end of file

make sure linting is in order, see http://pandas-docs.github.io/pandas-docs-travis/contributing.html#python-pep8

further I'd like to see you add mypy very similar to how linting works, and have it pass (on specifically mentioned files). That way we can incrementaly add things.

If you show me the commands you are running I can add this (to master), just to get things started.
(include how you are installing).

codecov · 2017-04-02T14:51:38Z

Codecov Report

Merging #15866 into master will decrease coverage by 0.01%.
The diff coverage is 95.83%.

@@            Coverage Diff             @@
##           master   #15866      +/-   ##
==========================================
- Coverage   90.98%   90.96%   -0.02%     
==========================================
  Files         143      144       +1     
  Lines       49449    49457       +8     
==========================================
- Hits        44993    44991       -2     
- Misses       4456     4466      +10

Flag	Coverage Δ
#multiple	`88.72% <95.83%> (-0.01%)`	⬇️
#single	`40.66% <62.5%> (-0.1%)`	⬇️

Impacted Files	Coverage Δ
pandas/types/hinting.py	`100% <100%> (ø)`
pandas/core/base.py	`95.53% <94.44%> (+0.02%)`	⬆️
pandas/io/gbq.py	`25% <0%> (-58.34%)`	⬇️
pandas/core/common.py	`90.96% <0%> (-0.34%)`	⬇️
pandas/core/frame.py	`97.56% <0%> (-0.1%)`	⬇️

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 d1e1ba0...95f7039. Read the comment docs.

codecov · 2017-04-02T14:51:46Z

Codecov Report

Merging #15866 into master will increase coverage by 0.57%.
The diff coverage is 95.83%.

@@            Coverage Diff             @@
##           master   #15866      +/-   ##
==========================================
+ Coverage   90.39%   90.96%   +0.57%     
==========================================
  Files         161      144      -17     
  Lines       50863    49457    -1406     
==========================================
- Hits        45977    44991     -986     
+ Misses       4886     4466     -420

Flag	Coverage Δ
#multiple	`88.72% <95.83%> (+0.56%)`	⬆️
#single	`40.66% <62.5%> (+0.23%)`	⬆️

Impacted Files	Coverage Δ
pandas/types/hinting.py	`100% <100%> (ø)`
pandas/core/base.py	`95.53% <94.44%> (-0.65%)`	⬇️
pandas/io/gbq.py	`25% <0%> (-58.34%)`	⬇️
pandas/tools/plotting.py	`71.77% <0%> (-10.05%)`	⬇️
pandas/types/concat.py	`98.06% <0%> (-1.94%)`	⬇️
pandas/conftest.py	`94.11% <0%> (-1.72%)`	⬇️
pandas/compat/pickle_compat.py	`68.29% <0%> (-1.22%)`	⬇️
pandas/tools/hashing.py	`99.02% <0%> (-0.98%)`	⬇️
pandas/io/json/table_schema.py	`95.58% <0%> (-0.19%)`	⬇️
... and 189 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 80abd97...76126f2. Read the comment docs.

jreback · 2017-04-02T14:52:09Z

pandas/types/hinting.py

@@ -0,0 +1,10 @@
+#!/usr/bin/env python


you don't need this top line, this is not an executable file

good catch. i must've clicked the wrong file template

jreback · 2017-04-02T15:01:24Z

@brmc ok I rebase and pushed a couple of commits to auto check this on travis (it fails now but the check is in place).

The idea is to make this pass cleanly (and check what you have so far). Then can add other things.

Note that you will need to rebase this pretty much every time you push (just in case). As the code base does move.

jreback · 2017-04-02T15:10:36Z

pandas/core/base.py

@@ -1,10 +1,13 @@
 """
 Base and utility classes for pandas objects.
 """
+from typing import Any, Callable, Optional, Tuple, Union


instead of doing this directly, move this to pandas.types.hinting.py and import from there. IOW that will be the 1 place that we inherit typing things from.

we can then handle the compat issues when typing is not installed (on 2.7).

jreback · 2017-04-02T15:11:16Z

pandas/core/base.py

 from pandas import compat
 from pandas.compat import builtins
 import numpy as np

+from pandas.types.hinting import ArrayLike, PythonScalar, Scalar, Buffer


I am actually ok in doing

from pandas.types.hinting import * # noqa (you need the noqa because of the * import.

easier to type I think as well.

jreback · 2017-04-18T22:20:25Z

@brmc love for you to update this if you can.

TomAugspurger · 2017-04-26T01:53:35Z

@brmc I'm putting a bit of time into this to see if I can get core/base.py passing. It'd be nice to get one file going, and then add chunks in smaller PRs afterwards if you're interested. That'll help with rebasing.

brmc · 2017-04-27T18:21:10Z

@TomAugspurger that sounds good. I'm not nearly as able to invest as much time into this as I originally planned when I started which is probably pretty frustrating for the core team. but yea, i'll help out when I can

TomAugspurger · 2017-04-27T18:24:27Z

Cool. I'll try to finish off base.py, tonight or tomorrow, and I'll cc you on future issues!

CLN: created foundation for complex type annotations (GH14468) This is mostly just a stub file for now until a more clear picture develops What has been noticed so far: * numpy has no clear definition of what array_like means. all of these are valid: - Python scalars - tuples - lists - buffers - scalars in both python and numpy - more? * similar story but not so extreme with dtypes * python and numpy scalar helpers have been defined CLN: annotated IndexOpsMixin (GH14468) CLN: fixed a couple mistakes in IndexOpsMixin (GH14468) CLN: cleaned up some import statements and reverted a file commited by accident (GH14468) CLN: temporary work around for buffer error in python3 (GH14468) CLN: temporary work around for buffer error in python3 part 2 (GH14468) add trivial mypy check Fixup

teh · 2017-05-08T20:42:21Z

ci/install_travis.sh

@@ -114,6 +114,10 @@ if [ "$LINT" ]; then
   pip install cpplint
 fi

+if [ "$TYPING" ]; then
+   pip install mypy-lang


mypy-lang is now called just mypy. I'll update in my branch.

TomAugspurger · 2017-05-08T21:02:35Z

Ok, some review would be appreciated, especially from anyone who's actually familiar with mypy :)

CI / package stuff

We'll put all of our custom types / shorthand in pandas/types/hinting.py
All the configuration should go in setup.cfg
Use sh ci/typing.sh to run mypy on the currently typed files. Currently it's base.py and we'll add more as we go

General Notes

I lost my original file with notes, so this is from memory:

I added quite a few Any types that are too general, like https://github.com/pandas-dev/pandas/pull/15866/files#diff-9c718a39eb63c1bfac4fbeacf2906ffdR430 I think we can make these stricter
I had to add more ABCs in pandas/core/dtypes/generic.py mainly for mixins. Otherwise we get a bunch of warnings about the Mixin class using attributes it doesn't have. ABCs are how the mypy devs recommend getting around this
I'm not sure how best to document our custom types, like SelectionFunction (the thing passed to {groupby/rolling/window/ndframe}.agg

jreback · 2017-06-10T19:05:24Z

can you rebase and update?

teh

Nice!

teh · 2017-06-16T23:02:26Z

pandas/core/base.py

@@ -92,14 +116,17 @@ def __unicode__(self):
        return object.__repr__(self)

    def _dir_additions(self):
+        # type: () -> typing.Set[str]


In what cases would we use str over Text (see __unicode__) in type annotations?

Reading the docs [1] I think what you are doing here is probably correct because str is python 2/3 implementation specific but I'm not 100% sure.

[1]
https://docs.python.org/3/library/typing.html#typing.Text

TomAugspurger · 2017-06-20T22:30:27Z

FWIW, I think this branch is pretty much abandoned (by me anyway) This version won't even import, since our pandas ABCs behave differently that real metaclasses. Starting with the base classes was probably a mistake. I think the Dropbox team adding type annotations recommend starting with the leaves of a library, and working your we into the base. If anyone wants to pick it up, I've put the cherry-picked the CI related things to https://github.com/TomAugspurger/pandas/tree/annotations-2 (and I guess a start of the hashing module, though that'll probably fall behind relatively quickly)

…

On Fri, Jun 16, 2017 at 6:06 PM, teh ***@***.***> wrote: ***@***.**** approved this pull request. Nice! ------------------------------ In pandas/core/base.py <#15866 (comment)>: > @@ -92,14 +116,17 @@ def __unicode__(self): return object.__repr__(self) def _dir_additions(self): + # type: () -> typing.Set[str] In what cases would we use str over Text (see __unicode__) in type annotations? Reading the docs [1] I think what you are doing here is probably correct because str is python 2/3 implementation specific but I'm not 100% sure. [1] https://docs.python.org/3/library/typing.html#typing.Text — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#15866 (review)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABQHIi2-Kg-CbjciOatYC2UHaXZ2XE7dks5sEwpygaJpZM4Mwtae> .

jreback · 2017-06-21T00:58:29Z

ok let's close then.

brmc changed the title ~~CLN GH14468: trial pull-request to make sure everything is in order before proceeding~~ CLN: [WIP] trial pull-request to make sure everything is in order before proceeding (GH14468) Apr 2, 2017

jreback added the Code Style Code style, linting, code_checks label Apr 2, 2017

jreback reviewed Apr 2, 2017

View reviewed changes

jreback force-pushed the master branch from 95f7039 to 45ff047 Compare April 2, 2017 15:00

jreback reviewed Apr 2, 2017

View reviewed changes

TomAugspurger mentioned this pull request May 8, 2017

adding static type checking with mypy #14468

Closed

TomAugspurger force-pushed the master branch from 7220db5 to fbc3543 Compare May 8, 2017 20:22

TomAugspurger added 2 commits May 8, 2017 15:28

revert algos

463ef16

CFG: Put config in setup.cfg

d606649

teh reviewed May 8, 2017

View reviewed changes

TomAugspurger added 2 commits May 8, 2017 15:46

DOC: Start docs on mypy

0bf7a19

COMPAT: Catch ImportError from hinting

76126f2

teh approved these changes Jun 16, 2017

View reviewed changes

jreback closed this Jun 21, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CLN: [WIP] trial pull-request to make sure everything is in order before proceeding (GH14468) #15866

CLN: [WIP] trial pull-request to make sure everything is in order before proceeding (GH14468) #15866

brmc commented Apr 2, 2017

jreback commented Apr 2, 2017

codecov bot commented Apr 2, 2017

codecov bot commented Apr 2, 2017 •

edited

Loading

jreback Apr 2, 2017

brmc Apr 23, 2017

jreback commented Apr 2, 2017

jreback Apr 2, 2017

jreback Apr 2, 2017

jreback commented Apr 18, 2017

TomAugspurger commented Apr 26, 2017

brmc commented Apr 27, 2017

TomAugspurger commented Apr 27, 2017

teh May 8, 2017

TomAugspurger commented May 8, 2017

jreback commented Jun 10, 2017

teh left a comment

teh Jun 16, 2017

TomAugspurger commented Jun 20, 2017 via email

jreback commented Jun 21, 2017

CLN: [WIP] trial pull-request to make sure everything is in order before proceeding (GH14468) #15866

CLN: [WIP] trial pull-request to make sure everything is in order before proceeding (GH14468) #15866

Conversation

brmc commented Apr 2, 2017

jreback commented Apr 2, 2017

codecov bot commented Apr 2, 2017

Codecov Report

codecov bot commented Apr 2, 2017 • edited Loading

Codecov Report

jreback Apr 2, 2017

Choose a reason for hiding this comment

brmc Apr 23, 2017

Choose a reason for hiding this comment

jreback commented Apr 2, 2017

jreback Apr 2, 2017

Choose a reason for hiding this comment

jreback Apr 2, 2017

Choose a reason for hiding this comment

jreback commented Apr 18, 2017

TomAugspurger commented Apr 26, 2017

brmc commented Apr 27, 2017

TomAugspurger commented Apr 27, 2017

teh May 8, 2017

Choose a reason for hiding this comment

TomAugspurger commented May 8, 2017

CI / package stuff

General Notes

jreback commented Jun 10, 2017

teh left a comment

Choose a reason for hiding this comment

teh Jun 16, 2017

Choose a reason for hiding this comment

TomAugspurger commented Jun 20, 2017 via email

jreback commented Jun 21, 2017

codecov bot commented Apr 2, 2017 •

edited

Loading