Refactor timezones functions out of tslib
State: closed

`_libs.tslib` is over 5k lines and is imported by a bunch of other modules including `_libs.lib`.  It looks like it was pasted together from a bunch of older files.  There are a handful of areas where significant chunks can be refactored out in complexity-reducing (and testability-increasing) ways.  This is the first one: timezones.  (Next up: parsing (and gathering parsing code dispersed across pandas))

timezones.pyx has no other dependencies within pandas, helping to de-tangle some of the `_libs` modules

Code in timezones is used in both `_libs.tslib` and `_libs.period`, and a bit in `_libs.index`.

This is one of several steps in making _libs.period not need to import from non-cython code and
ideally not need to import `tslib` (though `NaT` makes that tough).  See existing comments
in `_libs.__init__` on why this is desireable.

This is the first of several independent pieces to be split off of `tslib`.

There are also several notes on functions that appear to be unused and may be ready for removal.

Removes `datetime_helper` dependency from most of `_libs`, as it is somehow slower than a plain cython version.  In cases where C can be replaced by idiomatic cython without hurting performance, I'm calling that a win.

- [ ] closes #xxxx
- [ ] tests added / passed
- [ ] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry


Labels:
name: Internals
description: Related to non-user accessible pandas implementation
name: Clean

Comments:
Hello @jbrockmendel! Thanks for updating the PR.



 - In the file [`setup.py`](https://github.com/pandas-dev/pandas/blob/0540e7733828bc0a587fa23c87ead30d285c16d1/setup.py), following are the PEP8 issues :

> [Line 487:50](https://github.com/pandas-dev/pandas/blob/0540e7733828bc0a587fa23c87ead30d285c16d1/setup.py#L487): [E502](https://duckduckgo.com/?q=pep8%20E502) the backslash is redundant between brackets
> [Line 493:51](https://github.com/pandas-dev/pandas/blob/0540e7733828bc0a587fa23c87ead30d285c16d1/setup.py#L493): [E502](https://duckduckgo.com/?q=pep8%20E502) the backslash is redundant between brackets
> [Line 502:52](https://github.com/pandas-dev/pandas/blob/0540e7733828bc0a587fa23c87ead30d285c16d1/setup.py#L502): [E502](https://duckduckgo.com/?q=pep8%20E502) the backslash is redundant between brackets



##### Comment last updated on August 28, 2017 at 17:29 Hours UTC
> Removes datetime_helper dependency from most of _libs, as it is somehow slower than a plain cython version.

Do you have any (empirical) timing information to support this statement?
> Do you have any (empirical) timing information to support this statement?

Brief discussion of profiling results [here](https://github.com/pandas-dev/pandas/issues/17234#issuecomment-322932822).
Regarding the `total_seonds` in `datetime_helper.h`, a comment mentions there it is for 2.6 compat, so it can maybe be removed altogether? (without a cython replacement)
> [...] so it can maybe be removed altogether?

That would be excellent.  If there's a concensus, I'll amend the PR.
> That would be excellent. If there's a concensus, I'll amend the PR.

You will need to check that it is actually possible, but I suppose it will. `timedelta.total_seconds()` was only added in python 2.7, but since we dropped 2.6 support long ago, we can just use the `total_seconds` method instead of our custom function.
> but since we dropped 2.6 support long ago, we can just use the total_seconds method instead of our custom function.

@jorisvandenbossche I'll give that a try and see how it goes.  Is it possible/likely that this would also explain why _libs/src/datetime/np_datetime.c duplicates a bunch of stuff from numpy 1.7?  i.e. at a time when pre-1.7 versions of numpy were supported?
Getting rid of `total_seconds` (and `tot_seconds`) in all of the pyx files works.  There is one call from objToJSON.c that someone else should take a look at.
pls pls pls limit the changes to the minimal set that are required. mixing changes makes this much harder to review.
# [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/17274?src=pr&el=h1) Report
> Merging [#17274](https://codecov.io/gh/pandas-dev/pandas/pull/17274?src=pr&el=desc) into [master](https://codecov.io/gh/pandas-dev/pandas/commit/2bec750b21b8715e3f55e71a6c69f2abef54d08b?src=pr&el=desc) will **decrease** coverage by `0.01%`.
> The diff coverage is `n/a`.

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

```diff
@@            Coverage Diff             @@
##           master   #17274      +/-   ##
==========================================
- Coverage   91.01%   90.99%   -0.02%     
==========================================
  Files         162      162              
  Lines       49558    49558              
==========================================
- Hits        45105    45096       -9     
- Misses       4453     4462       +9
```

| Flag | Coverage Δ | |
|---|---|---|
| #multiple | `88.77% <ø> (ø)` | :arrow_up: |
| #single | `40.25% <ø> (-0.07%)` | :arrow_down: |

| [Impacted Files](https://codecov.io/gh/pandas-dev/pandas/pull/17274?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pandas/io/gbq.py](https://codecov.io/gh/pandas-dev/pandas/pull/17274?src=pr&el=tree#diff-cGFuZGFzL2lvL2dicS5weQ==) | `25% <0%> (-58.34%)` | :arrow_down: |
| [pandas/core/frame.py](https://codecov.io/gh/pandas-dev/pandas/pull/17274?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvZnJhbWUucHk=) | `97.72% <0%> (-0.1%)` | :arrow_down: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/17274?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/17274?src=pr&el=footer). Last update [2bec750...1a62298](https://codecov.io/gh/pandas-dev/pandas/pull/17274?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/17274?src=pr&el=h1) Report
> Merging [#17274](https://codecov.io/gh/pandas-dev/pandas/pull/17274?src=pr&el=desc) into [master](https://codecov.io/gh/pandas-dev/pandas/commit/473a7f3c186f6b0bfd9d3ce413fb627cf7a8f111?src=pr&el=desc) will **decrease** coverage by `0.04%`.
> The diff coverage is `n/a`.

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

```diff
@@            Coverage Diff             @@
##           master   #17274      +/-   ##
==========================================
- Coverage   91.03%   90.99%   -0.05%     
==========================================
  Files         162      162              
  Lines       49567    49567              
==========================================
- Hits        45125    45104      -21     
- Misses       4442     4463      +21
```

| Flag | Coverage Δ | |
|---|---|---|
| #multiple | `88.77% <ø> (-0.03%)` | :arrow_down: |
| #single | `40.24% <ø> (-0.07%)` | :arrow_down: |

| [Impacted Files](https://codecov.io/gh/pandas-dev/pandas/pull/17274?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pandas/io/gbq.py](https://codecov.io/gh/pandas-dev/pandas/pull/17274?src=pr&el=tree#diff-cGFuZGFzL2lvL2dicS5weQ==) | `25% <0%> (-58.34%)` | :arrow_down: |
| [pandas/plotting/\_converter.py](https://codecov.io/gh/pandas-dev/pandas/pull/17274?src=pr&el=tree#diff-cGFuZGFzL3Bsb3R0aW5nL19jb252ZXJ0ZXIucHk=) | `63.23% <0%> (-1.82%)` | :arrow_down: |
| [pandas/core/frame.py](https://codecov.io/gh/pandas-dev/pandas/pull/17274?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvZnJhbWUucHk=) | `97.72% <0%> (-0.1%)` | :arrow_down: |
| [pandas/core/indexing.py](https://codecov.io/gh/pandas-dev/pandas/pull/17274?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvaW5kZXhpbmcucHk=) | `93.94% <0%> (ø)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/17274?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/17274?src=pr&el=footer). Last update [473a7f3...0540e77](https://codecov.io/gh/pandas-dev/pandas/pull/17274?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

Gonna push on this a little bit.  While #17363 and #17342 are "just" about making code more self-contained and maintainable, this PR is a blocker for some more performance-relevant fixes.

In particular, the end goal is to move `tseries.offsets.DateOffset` and `tseries.frequencies.to_offset` into cython, avoid the runtime import of `to_offset`, and make `DateOffset` immutable.  Being able to import timezone functions without importing all of `tslib` makes it possible to move large chunks of `frequencies` and `offsets` into cython.
> I want to see an asv run of all timezone & time realted benchmarks. (IOW show the diffs)

asv is complaining about build errors, so I'll put together a handful of measurements just using %timeit.  There isn't a Timestamp/timezone-specific benchmark file,  though I think there is one added in #17331 
> asv is complaining about build errors, so I'll put together a handful of measurements just using %timeit. There isn't a Timestamp/timezone-specific benchmark file, though I think there is one added in #17331

pls don't. we use asv for a reason to avoid ad-hoc things like this.
> pls don't. we use asv for a reason to avoid ad-hoc things like this.

OK... what's the difference between this and the other times you've told me to use %timeit?

```
asv continuous -f 1.1 -E virtualenv HEAD -b timeseries.py
· Creating environments
· Discovering benchmarks
·· Uninstalling from virtualenv-py2.7-Cython-matplotlib-numexpr-numpy-openpyxl-pytest-scipy-sqlalchemy-tables-xlrd-xlsxwriter-xlwt
·· Installing into virtualenv-py2.7-Cython-matplotlib-numexpr-numpy-openpyxl-pytest-scipy-sqlalchemy-tables-xlrd-xlsxwriter-xlwt..
·· Error running ~/pd/tslibs-timezones/pandas/asv_bench/env/dbb8113eac34359a9ef26b93e54a1d03/bin/python ~/.local/lib/python2.7/site-packages/asv/benchmark.py discover ~/pd/tslibs-timezones/pandas/asv_bench/benchmarks /tmp/tmpUDGky2
             STDOUT -------->
             
             STDERR -------->
             Traceback (most recent call last):
               File "~/.local/lib/python2.7/site-packages/asv/benchmark.py", line 867, in <module>
                 commands[mode](args)
               File "~/.local/lib/python2.7/site-packages/asv/benchmark.py", line 812, in main_discover
                 list_benchmarks(benchmark_dir, fp)
               File "~/.local/lib/python2.7/site-packages/asv/benchmark.py", line 797, in list_benchmarks
                 for benchmark in disc_benchmarks(root):
               File "~/.local/lib/python2.7/site-packages/asv/benchmark.py", line 696, in disc_benchmarks
                 for module in disc_files(root, os.path.basename(root) + '.'):
               File "~/.local/lib/python2.7/site-packages/asv/benchmark.py", line 648, in disc_files
                 module = import_module(package + filename)
               File "/usr/lib/python2.7/importlib/__init__.py", line 37, in import_module
                 __import__(name)
               File "~/pd/tslibs-timezones/pandas/asv_bench/benchmarks/index_object.py", line 1, in <module>
                 from .pandas_vb_common import *
               File "~/pd/tslibs-timezones/pandas/asv_bench/benchmarks/pandas_vb_common.py", line 1, in <module>
                 from pandas import *
               File "~/pd/tslibs-timezones/pandas/asv_bench/env/dbb8113eac34359a9ef26b93e54a1d03/local/lib/python2.7/site-packages/pandas/__init__.py", line 35, in <module>
                 "the C extensions first.".format(module))
             ImportError: C extension: No module named tslibs.timezones not built. If you want to import pandas from the source directory, you may need to run 'python setup.py build_ext --inplace --force' to build the C extensions first.
```

Same deal with py3.5 except it's complaining about `period.pquarter`
this ian changing way more not-easy-to-inspect code differences. it's very easy to lose perf without constant benchmarking

if you are going to change significant code then get asv to work
it should be quite straightforward 

i have never used this with virtual env can't say if it actually works properly (use conda)
The same error is occuring with conda.  The same "ImportError: C extension: No module named tslibs.timezones not built." is occuring even if I just run `asv continuous -f 1.1 master -b ^timeseries` where there _should be no_ tslibs.timezones.  It's 100° out and I'm not inclined to spend any more time wrestling with a tool that I'm yet to be convinced gives meaningful measurements in the first place.

I'll make a series of PRs that move these functions over one at a time.
closing this as its replaced by other PR's. let's close older / extraneous PR's.