CLN: reorg type inference & introspection
State: closed

closes #12503 

So this is the big boy. destroyed `core/common.py` and split up. Now have no more import issues and a nice private API. Nothing is exported by default. You have to explicity import things.

There is now a `pandas.api`, so:

```
In [1]: from pandas import api

In [2]: dir(api)
Out[2]: 
['__builtins__',
 '__doc__',
 '__file__',
 '__name__',
 '__package__',
 '__path__',
 'is_any_int_dtype',
 'is_bool',
 'is_bool_dtype',
 'is_categorical',
 'is_categorical_dtype',
 'is_complex',
 'is_complex_dtype',
 'is_datetime64_any_dtype',
 'is_datetime64_dtype',
 'is_datetime64_ns_dtype',
 'is_datetime64tz_dtype',
 'is_datetimetz',
 'is_dtype_equal',
 'is_extension_type',
 'is_float',
 'is_float_dtype',
 'is_floating_dtype',
 'is_int64_dtype',
 'is_integer',
 'is_integer_dtype',
 'is_number',
 'is_numeric_dtype',
 'is_object_dtype',
 'is_scalar',
 'is_sparse',
 'is_string_dtype',
 'is_timedelta64_dtype',
 'is_timedelta64_ns_dtype',
 'np',
 'pandas_dtype',
 'types']
```

Previously `pandas.core.common` was a huge namespace of everything under the sun. As a result, you often had to do `com.` imports to do stuff, and sometimes import inside a function.

These are essentially hierarchically arrange in order of deps.
- `pandas.types`
  - `generic` : `ABC*` class infererence, no deps
  - `dtypes` : extension dtypes, no deps
  - `inference` : scalar / and `is_list_list` instrospection, no deps
  - `common` : imports all from `inference` (for convenience), adds all `is_*_dtype` things. deps on dtypes, generic, inference
  - `missing` : `isnull`, `notnull`, `array_equivalent` : deps on common
  - `cast` :  the old `core.convert` and all types of inference / conversions that littered core.common, deps on common. Mostly this is private stuff that Series/DataFrame uses (e.g. most methods are `_` leading)

`pandas.core.common` now doesn't depend on anything else.

So now can easily write:

`from pandas.types.common import is_integer` or whatever at the top of any file and it will just work. Further almost all references to `com` gone.

This only change was I added `array_equivalent` to the `pd.` namespace, as formerly it was accessible as by:

```
from pandas.core.common import array_equivalent
```

There _should_ be NO user facing changes at all, but I will put a note that warns if people were using the 'private' API (IOW if they were directly importing stuff), then things have moved.

@wesm @jorisvandenbossche @shoyer @TomAugspurger @sinhrks 


Labels:
name: Clean

Comments:
There is a slightly problematic aspect here -- other libraries that needed to be able to use `is_categorical_dtype` (I'm guilty of this: https://github.com/wesm/feather/blob/master/python/feather/ext.pyx#L83) -- it was in one place, and now it's in another. I get that this wasn't necessarily a public API -- is there some other way to get at this information, like `s.dtype == 'category'`?

no that type of dtype comparison isn't safe

i can simply import some things back into the pdcom space
though am trying to limit what we import 

One safe option (the best we came up, if I recall correctly) is hasattr(s,
'cat')

On Wed, May 11, 2016 at 7:15 PM, Jeff Reback notifications@github.com
wrote:

> no that type of dtype comparison isn't safe
> 
> i can simply import some things back into the pdcom space
> though am trying to limit what we import
> 
> —
> You are receiving this because you were mentioned.
> Reply to this email directly or view it on GitHub
> https://github.com/pydata/pandas/pull/13147#issuecomment-218641967

cc @llllllllll 
are you guys using the 'private' API here?

It might be worth defining a public API for this purpose. Of course the ship has sailed on earlier versions of pandas

These are also added back to `pandas.core.common` (for backwards compat at least). I am not sure how to even go about deprecating these as this would require a module level deprecation on the import (IIRC this is possible as I think @njsmith did it somewhere)..

```
In [2]: from pandas.api import types as pt

In [3]: dir(pt)
Out[3]: 
['__builtins__',
 '__doc__',
 '__file__',
 '__name__',
 '__package__',
 '__path__',
 'is_any_int_dtype',
 'is_bool',
 'is_bool_dtype',
 'is_categorical',
 'is_categorical_dtype',
 'is_complex',
 'is_complex_dtype',
 'is_datetime64_any_dtype',
 'is_datetime64_dtype',
 'is_datetime64_ns_dtype',
 'is_datetime64tz_dtype',
 'is_datetimetz',
 'is_dtype_equal',
 'is_extension_type',
 'is_float',
 'is_float_dtype',
 'is_floating_dtype',
 'is_int64_dtype',
 'is_integer',
 'is_integer_dtype',
 'is_number',
 'is_numeric_dtype',
 'is_object_dtype',
 'is_scalar',
 'is_sparse',
 'is_string_dtype',
 'is_timedelta64_dtype',
 'is_timedelta64_ns_dtype',
 'np',
 'pandas_dtype']
```

@jreback the only things we have in Zipline that are touching pandas.core are in a utility module that's already doing backwards-compat stuff anyway:

```
git --no-pager grep -n -e pandas.core -- 
zipline/lib/_factorize.pyx:100:        # This is all adapted from pandas.core.algorithms.factorize.
zipline/utils/munge.py:15:from pandas.core.common import mask_missing
zipline/utils/munge.py:17:    from pandas.core.common import backfill_2d, pad_2d
zipline/utils/munge.py:19:    # In 0.17, pad_2d and backfill_2d werw moved from pandas.core.common to
zipline/utils/munge.py:20:    # pandas.core.missing
zipline/utils/munge.py:21:    from pandas.core.missing import backfill_2d, pad_2d
```

@jreback See https://github.com/njsmith/metamodule. It requires some messy tricks to support Python <3.5 though.

hmm, that might do the trick of deprecating these de-facto apis (though prob will do that in the 0.19.0).

The messy tricks aren't too bad because they only have to support a specific list of old, non-moving Python versions, and I already went through and checked that all of those versions are handled correctly. All future versions of Python have a clean upstream supported way to do this.

But, if you want to avoid that, you actually can -- the really clever stuff in meta module is required to support the case where you need to fix up a module from inside that module (e.g. if you wanted to deprecate to attribute of pandas itself), without rewriting all your API export logic. For pandas.core.common you could have pandas.core import it and then replace it with a metamodule-style subclass without getting ctypes involved.

More generally, what do people think about having an explicit contract for what's in the public vs private API? 

> More generally, what do people think about having an explicit contract for what's in the public vs private API?

:+1:

I don't know if it's stated explicitly, but everything in `pandas.core` that isn't exported to the `pandas` namespace is private API. The problem here is that private stuff is useful, so people have used it anyways!

any comments 
@jorisvandenbossche @shoyer @wesm @sinhrks @TomAugspurger 

Tried to do a github search for where pandas.core.common was used, but it's mostly just a bunch of people's virtualenvs with pandas. Did find
- from `pandas.core.common import PandasError` (https://github.com/pydata/pandas-datareader/blob/b83ae293175782698089beb0ba1dd557952bfecb/pandas_datareader/_utils.py) in pandas data-reader, so should give them a headsup

No uses of pandas.core.common in seaborn or statsmodels, so that's good.

yeah I added back the public stuff anyhow (though we should maybe use the meta-module tricks to deprecate it in 0.19.0).

> Tried to do a github search for where pandas.core.common was used, but it's mostly just a bunch of people's virtualenvs with pandas

There's some tooling here to work around this obnoxious feature of GH search, in case you find it useful: https://github.com/njsmith/codetrawl/

Example: https://github.com/njsmith/ufunc-abi-analysis

any other comments.

Certainly +1 on the general idea! I think it is important to better formulate which functions can be relied on in other packages. 
Some related issues discussing similar ideas: #5460, #9895

But some comments ot the PR:
- Moving all those functions from `core` to `types` seems a bit strange to me. These are a lot of common utility functions, which are not necessarily tied to `types` (I would keep that for the actual custom dtypes and related functionality we have in pandas). Having a `pd.common` or `pd.utils` module holding these seems more logical to me.
- I am not fully convinced of the `pandas.api` module (certainly the `pd.api.types` seems unnecessary as there is already a `pd.types.api`). But, I personally think we should try to use the same api internally in pandas as how we promote to use it in other packages using pandas.
- we should test the api somewhere (whether it is just testing that those functions are available in a certain namespace, so we get notified by a failing test when a function gets removed for some reason)
- we should also document it somewhere which functions are regarded as public (eg a new section in the docs for "package development depending on pandas" or something better phrased)
- I think this should not necessarily need to go in a minor release 0.18.2 (as I said in https://github.com/pydata/pandas/issues/13003#issuecomment-218792295). I know it is mainly copy pasting and not actually changing implementation, but it is still touching a lot of lines. I am also not saying we should hold this up, but it can just be a matter of naming and we can call master 0.19.0. Anyway, I should find the time to reply to you in https://github.com/pydata/pandas/issues/13003. This is not directly related to the content of this PR, maybe best to leave that discussion there.

@jreback small question, above you said "I added array_equivalent to the pd. namespace", is this still the case? (diff is so huge it is difficult to check) If so, is this needed? (internally it is imported from `pandas.types.missing`)

@jorisvandenbossche some responses

> Moving all those functions from core to types seems a bit strange to me. These are a lot of common utility functions, which are not necessarily tied to types (I would keep that for the actual custom dtypes and related functionality we have in pandas). Having a pd.common or pd.utils module holding these seems more logical to me.

the vast majority of this IS type stuff. The problem is we had a circular import issue where things depended on `core.common` which then depended on that (e.g. `core.base`); here I removed that dependency.

> I am not fully convinced of the pandas.api module (certainly the pd.api.types seems unnecessary as there is already a pd.types.api). But, I personally think we should try to use the same api internally in pandas as how we promote to use it in other packages using pandas.

I did this for namespace compat, IOW. then it allows a 'user' to only import think rather than having to import the entire api e.g. `from pandas.api import testing, types` for example, to allow specific imports.

> we should test the api somewhere (whether it is just testing that those functions are available in a certain namespace, so we get notified by a failing test when a function gets removed for some reason)

yes I will add this

> we should also document it somewhere which functions are regarded as public (eg a new section in the docs for "package development depending on pandas" or something better phrased)

yes I should add this, though sort of the reason for including now (aside from its a cleanup) is that we don't have to fully publish this till 0.19.0

> @jreback small question, above you said "I added array_equivalent to the pd. namespace", is this still the case? (diff is so huge it is difficult to check) If so, is this needed? (internally it is imported from pandas.types.missing)

yeah I thought we exposed this, but going to remove it from the `pd` namespace; its not necessary

@jorisvandenbossche I think this [commit](https://github.com/pydata/pandas/pull/13147/commits/e34d919edd6fc214efc77f3ce7bf875dc2d73a26) should help with the `pandas.*` api (and `pandas.api`) going forward.

appreciate ya'll taking a look.

@sinhrks @jorisvandenbossche @sinhrks @TomAugspurger @wesm 

this doesn't change very much of the public API (though establishes a minimal `pandas.api`); mainly now a reorg of things and splitting up common.py to public/private.

This is now becoming harder to rebase, so pls comment soon.

## [Current coverage](https://codecov.io/gh/pydata/pandas/pull/13147?src=pr) is **84.39%**

> Merging [#13147](https://codecov.io/gh/pydata/pandas/pull/13147?src=pr) into [master](https://codecov.io/gh/pydata/pandas/branch/master?src=pr) will increase coverage by **0.05%**

``` diff
@@             master     #13147   diff @@
==========================================
  Files           138        142     +4   
  Lines         51095      51218   +123   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits          43092      43224   +132   
+ Misses         8003       7994     -9   
  Partials          0          0          
```

> Powered by [Codecov](https://codecov.io?src=pr). Last updated by [27d2915...da94f4f](https://codecov.io/gh/pydata/pandas/compare/27d29158780bc7127bd944fc41eed3b74f38870b...da94f4fb1776c2cacb1d71beba09780372528a7d?src=pr)

ok this is rebased which is a PITA
practically every PR causes conflict with this so would be nice to push this in shortly

@jorisvandenbossche @wesm @sinhrks 

This seems fine to me, certainly an improvement over the status quo. For 0.19.0-dev, right? Hopefully we'll get some bug reports and we can either expand the API as needed.

yeah this is for 0.19.0

note that I left the the public API in pandas.core.common as well (eg the is_*)

should I take it out? I don't have an easy way of deprecating 

I think that's a good idea. To do a breaking release with no warning might cause some grumbling. This gives people an opportunity to migrate their APIs gradually without a fix-it-or-you-cant-upgrade.

@wesm ok I'll add that, but it will only be a doc-warning, unless we do some magic.

IOW not easy to even show a warning message on this 

```
from pandas.core.common import is_integer
```

As I said before, I dislike the introduction of `pandas.api` a bit as entry-point for external packages. I think we should have a good API that can be used both in external packages as in the different core pandas modules (to avoid discrepancies and confusion). Given that currently `pandas.api` is mainly the content of `pandas.types.common`, why not documenting it as available like that? (or putting it in `pandas.types` or `pandas.common` if we want it less deeply structured)
But given that I am the only one saying this, I will have to live with it :-)

Anyway, also given that this is indeed a pita to rebase, I am fine with merging this now. We can always adapt a little bit later (and once your PR is merged, it will also be easier to change some things further, indicating the value of this PR!)

> should I take it out? I don't have an easy way of deprecating

I agree we can't take it out (since it's already used in the wild). 
One way of being able to deprecate them would be to redefine them in core/common (which just calls the version in types/common) with a deprecation warning. That seems a lot of work, but I think it could maybe be done rather automatically in a for loop? 
(but also not essential to get this merged)