DEPR: Deprecate pandas.core.datetools
State: closed

Title is self-explanatory.  Closes #14094.


Labels:
name: Deprecate
description: Functionality to remove in pandas

Comments:
## [Current coverage](https://codecov.io/gh/pydata/pandas/pull/14105?src=pr) is 85.26% (diff: 78.26%)

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

``` diff
@@             master     #14105   diff @@
==========================================
  Files           139        140     +1   
  Lines         50562      50604    +42   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits          43116      43149    +33   
- Misses         7446       7455     +9   
  Partials          0          0          
```

> Powered by [Codecov](https://codecov.io?src=pr). Last update [59524af...2bb887d](https://codecov.io/gh/pydata/pandas/compare/59524af1e90d3dda2d885e711f7258704a020b6d...2bb887d0be90cea1f2980d6746556c5dada68ac1?src=pr)

Can you leave out the changes in the benchmarks? (Avoiding conflicts with my pr, i will incorporate changes there)

@jorisvandenbossche : Ah, I didn't see #14099.  Sure thing.  Once I get Travis to pass, I'll remove the commit.  In the meantime, I'll add a reminder to yours.

@jreback , @jorisvandenbossche : Travis is passing.  Ready to merge if there are no other concerns.

@jreback , @jorisvandenbossche : Travis is passing. Ready to merge if there are no other concerns.

need to add `__dir__` to `_DeprecatedModule` to get something reasonable, IOW the list of removals should be there and the namespace imported from the alts.

```
In [11]: pd.datetools.alts
Out[11]: 
frozenset({'pandas.tseries.frequencies',
           'pandas.tseries.offsets',
           'pandas.tseries.tools'})

In [12]: pd.datetools.removals
Out[12]: 
frozenset({'bday',
           'bmonthBegin',
           'bmonthEnd',
           'bquarterEnd',
           'businessDay',
           'byearEnd',
           'cbmonthBegin',
           'cbmonthEnd',
           'cday',
           'customBusinessDay',
           'customBusinessMonthBegin',
           'customBusinessMonthEnd',
           'day',
           'monthEnd',
           'quarterEnd',
           'week',
           'yearBegin',
           'yearEnd'})
```

further I am not sure this actually works

```
In [1]: dir(pandas.datetools)
Out[1]: 
['__class__',
 '__delattr__',
 '__dict__',
 '__doc__',
 '__format__',
 '__getattr__',
 '__getattribute__',
 '__hash__',
 '__init__',
 '__module__',
 '__new__',
 '__reduce__',
 '__reduce_ex__',
 '__repr__',
 '__setattr__',
 '__sizeof__',
 '__str__',
 '__subclasshook__',
 '__weakref__',
 'alts',
 'deprmod',
 'removals',
 'self_dir']

In [17]: from pandas.datetools import to_datetime
---------------------------------------------------------------------------
ImportError                               Traceback (most recent call last)
<ipython-input-17-014abe5066ec> in <module>()
----> 1 from pandas.datetools import to_datetime

ImportError: No module named datetools
```

something is wrong

pls add some tests to verify the actual deprecations themselves (e.g. that you can import the previous things, or a sample of them)

@jreback : Have you tried running `from pandas.datetools import to_datetime` on `master`?  It doesn't work either.  I don't think that's a problem.

@jreback : I don't want to overload `__dir__` as you described because then I can't differentiate methods that apart of the `class` itself and methods that are meant to be in `removals` OR `alts`.  That's the purpose of my first check in `__getattr__`.

@jreback : Don't fully understand your comments about testing.  Aren't I doing that already?

```
In [2]: pd.__version__
Out[2]: '0.18.1+425.gd26363b'

In [3]: dir(pd.datetools)[0:10]
Out[3]: 
['ABCDataFrame',
 'ABCIndexClass',
 'ABCSeries',
 'AmbiguousTimeError',
 'BDay',
 'BMonthBegin',
 'BMonthEnd',
 'BQuarterBegin',
 'BQuarterEnd',
 'BYearBegin']

```

@gfyoung you can easily override `__dir__`, you _know_ whats in `alt`/ `removals` (well you know as soon as you introspect them), which you can do lazily, e.g. when its needed

@jreback : Ah, that's a fair point.  I can just first check if it's in `dir` and then introspect.

yep. The ideal thing is to replicate as much as possible the existing behavior (and just show a depreaction warning).

@gfyoung The consequence of using a frozenset for the `alts` is that they have no longer a preferred order. For example, you now get the message to `Please use pandas.tseries.frequencies.xxx instead.` for many of the offsets, while in our docs / internals we import those from `pandas.tseries.offsets`

@jorisvandenbossche : That isn't a property `frozenset`, but rather one of `set` in particular.  `set` is unordered by definition.  However, your point about the "wrong" import being used is indicative of namespace pollution within each module.

What would you suggest I do then?

I didn't want to imply it was a consequence of the 'frozen' aspect of the sets :-). But initially you used a list I think? And performance was the reason to change it to set? Simply using a list would keep the preferred order. 
Personally, my preference in this case would go for correctness rather than performance (although what is correct is also debatable ... as you noted correctly the polluted namespaces between `frequencies` and `offsets`), `alts` is in this case a list/set of 3 elements, the `in` check for that will not be that critical here IMO

@jorisvandenbossche : It was a general performance consideration that @jreback brought up.  I originally used a `list`, but then he pointed out that `set` is faster.

While I do see your point about the imports and how they should come from specific places to be consistent with documentation, IMO the code is correct as is and shouldn't have to tailor to the namespace pollution that I pointed out.

@gfyoung its a bit more work, but you _can_ figure out where the imports are actually from, e.g. you can actually do the `from pandas.tseries.tools import *` (for each of the removals), then creating a mapping from the attr to the import. I know its a pain, but I think its necessary.

I actually did something like this (for some code I am working on which is old). I _know_ where things are, and it still took some time / trial and error to figure out the correct imports :)

not to mention the `monthEnd = MonthEnd()` is _really_ odd (though it _IS_ kind of like a singelton)

@jreback : `from <tseries module> import *` won't help since it still will import all other namespaces that are polluting it IINM.

I know it is not generic, but just using a list for `alts` instead of set works for this case. I don't see the need to make it more complicated than that.

Actually, can't we get the information from the object itself? (of course in this case where we want the full path it will give the right thing (frequencies or offsets), it will also not work generically for all imports where more top-level paths are used).

```
In [23]: getattr(pd.dateools, 'BDay')
Out[23]: pandas.tseries.offsets.BusinessDay
```

gives you that it should be 'offsets' and not frequencies

@jorisvandenbossche : How do you extract the full class path from this?

``` python
>>> getattr(pd.dateools, 'BDay')
<class 'pandas.tseries.offsets.BusinessDay'>
```

Perhaps there's an obvious way, but I don't see one.

If there isn't, then we might need to switch back to a `list` or the `OrderedSet` cookbook <a href="http://code.activestate.com/recipes/576694/">here</a> :smile: 

@jreback : Thoughts?

`__module__` gives me the path: 

```
In [40]: obj = getattr(pd.datetools, 'BDay')

In [41]: obj.__module__
Out[41]: 'pandas.tseries.offsets'
```

@jorisvandenbossche : That was indeed an obvious way.  Completely escaped my mind. :smile:

I am going to merge this, we can fix the actual depr warnings with correct modules in a follow-up PR, but then at least the deprecations are included in the rc