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

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

Labels:
name: Code Style
description: Code style, linting, code_checks

Comments:
```
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](https://codecov.io/gh/pandas-dev/pandas/pull/15866?src=pr&el=h1) Report
> Merging [#15866](https://codecov.io/gh/pandas-dev/pandas/pull/15866?src=pr&el=desc) into [master](https://codecov.io/gh/pandas-dev/pandas/commit/d1e1ba08ef259724ba71e0953c52e8e4ad81bd17?src=pr&el=desc) will **decrease** coverage by `0.01%`.
> The diff coverage is `95.83%`.

[![Impacted file tree graph](https://codecov.io/gh/pandas-dev/pandas/pull/15866/graphs/tree.svg?src=pr&width=650&token=eZ4WkYLtcO&height=150)](https://codecov.io/gh/pandas-dev/pandas/pull/15866?src=pr&el=tree)

```diff
@@            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%)` | :arrow_down: |
| #single | `40.66% <62.5%> (-0.1%)` | :arrow_down: |

| [Impacted Files](https://codecov.io/gh/pandas-dev/pandas/pull/15866?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pandas/types/hinting.py](https://codecov.io/gh/pandas-dev/pandas/pull/15866?src=pr&el=tree#diff-cGFuZGFzL3R5cGVzL2hpbnRpbmcucHk=) | `100% <100%> (ø)` | |
| [pandas/core/base.py](https://codecov.io/gh/pandas-dev/pandas/pull/15866?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvYmFzZS5weQ==) | `95.53% <94.44%> (+0.02%)` | :arrow_up: |
| [pandas/io/gbq.py](https://codecov.io/gh/pandas-dev/pandas/pull/15866?src=pr&el=tree#diff-cGFuZGFzL2lvL2dicS5weQ==) | `25% <0%> (-58.34%)` | :arrow_down: |
| [pandas/core/common.py](https://codecov.io/gh/pandas-dev/pandas/pull/15866?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvY29tbW9uLnB5) | `90.96% <0%> (-0.34%)` | :arrow_down: |
| [pandas/core/frame.py](https://codecov.io/gh/pandas-dev/pandas/pull/15866?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvZnJhbWUucHk=) | `97.56% <0%> (-0.1%)` | :arrow_down: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/15866?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/15866?src=pr&el=footer). Last update [d1e1ba0...95f7039](https://codecov.io/gh/pandas-dev/pandas/pull/15866?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
# [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/15866?src=pr&el=h1) Report
> Merging [#15866](https://codecov.io/gh/pandas-dev/pandas/pull/15866?src=pr&el=desc) into [master](https://codecov.io/gh/pandas-dev/pandas/commit/80abd97eb812a7bd87fa62909fdc041024567ffe?src=pr&el=desc) will **increase** coverage by `0.57%`.
> The diff coverage is `95.83%`.

[![Impacted file tree graph](https://codecov.io/gh/pandas-dev/pandas/pull/15866/graphs/tree.svg?width=650&height=150&src=pr&token=eZ4WkYLtcO)](https://codecov.io/gh/pandas-dev/pandas/pull/15866?src=pr&el=tree)

```diff
@@            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%)` | :arrow_up: |
| #single | `40.66% <62.5%> (+0.23%)` | :arrow_up: |

| [Impacted Files](https://codecov.io/gh/pandas-dev/pandas/pull/15866?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pandas/types/hinting.py](https://codecov.io/gh/pandas-dev/pandas/pull/15866?src=pr&el=tree#diff-cGFuZGFzL3R5cGVzL2hpbnRpbmcucHk=) | `100% <100%> (ø)` | |
| [pandas/core/base.py](https://codecov.io/gh/pandas-dev/pandas/pull/15866?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvYmFzZS5weQ==) | `95.53% <94.44%> (-0.65%)` | :arrow_down: |
| [pandas/io/gbq.py](https://codecov.io/gh/pandas-dev/pandas/pull/15866?src=pr&el=tree#diff-cGFuZGFzL2lvL2dicS5weQ==) | `25% <0%> (-58.34%)` | :arrow_down: |
| [pandas/tools/plotting.py](https://codecov.io/gh/pandas-dev/pandas/pull/15866?src=pr&el=tree#diff-cGFuZGFzL3Rvb2xzL3Bsb3R0aW5nLnB5) | `71.77% <0%> (-10.05%)` | :arrow_down: |
| [pandas/types/concat.py](https://codecov.io/gh/pandas-dev/pandas/pull/15866?src=pr&el=tree#diff-cGFuZGFzL3R5cGVzL2NvbmNhdC5weQ==) | `98.06% <0%> (-1.94%)` | :arrow_down: |
| [pandas/conftest.py](https://codecov.io/gh/pandas-dev/pandas/pull/15866?src=pr&el=tree#diff-cGFuZGFzL2NvbmZ0ZXN0LnB5) | `94.11% <0%> (-1.72%)` | :arrow_down: |
| [pandas/compat/pickle\_compat.py](https://codecov.io/gh/pandas-dev/pandas/pull/15866?src=pr&el=tree#diff-cGFuZGFzL2NvbXBhdC9waWNrbGVfY29tcGF0LnB5) | `68.29% <0%> (-1.22%)` | :arrow_down: |
| [pandas/tools/hashing.py](https://codecov.io/gh/pandas-dev/pandas/pull/15866?src=pr&el=tree#diff-cGFuZGFzL3Rvb2xzL2hhc2hpbmcucHk=) | `99.02% <0%> (-0.98%)` | :arrow_down: |
| [pandas/io/json/table\_schema.py](https://codecov.io/gh/pandas-dev/pandas/pull/15866?src=pr&el=tree#diff-cGFuZGFzL2lvL2pzb24vdGFibGVfc2NoZW1hLnB5) | `95.58% <0%> (-0.19%)` | :arrow_down: |
| ... and [189 more](https://codecov.io/gh/pandas-dev/pandas/pull/15866?src=pr&el=tree-more) | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/15866?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/15866?src=pr&el=footer). Last update [80abd97...76126f2](https://codecov.io/gh/pandas-dev/pandas/pull/15866?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

@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.
@brmc love for you to update this if you can.
@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.
@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
Cool. I'll try to finish off `base.py`, tonight or tomorrow, and I'll cc you on future issues!
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`
can you rebase and update?
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 <notifications@github.com> wrote:

> *@teh* approved this pull request.
>
> Nice!
> ------------------------------
>
> In pandas/core/base.py
> <https://github.com/pandas-dev/pandas/pull/15866#discussion_r122548736>:
>
> > @@ -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
> <https://github.com/pandas-dev/pandas/pull/15866#pullrequestreview-44680383>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/ABQHIi2-Kg-CbjciOatYC2UHaXZ2XE7dks5sEwpygaJpZM4Mwtae>
> .
>

ok let's close then.