Cross off a few tslibs-TODOs
State: closed

This is a hodge-podge addressing a few of the items in #17652.  Individual commits should be item-specific(ish).

- remove unused lib.arrmap
- de-privatize _checknull_with_nat
- fix a couple of C warnings caused by timedelta_struct
- remove Period._comparables



Labels:
name: Datetime
description: Datetime data dtype
name: Clean

Comments:
While you are touching this file I just noticed a typo in the below line:

https://github.com/jbrockmendel/pandas/blob/6ef9227fda1ee595eecf298e65ba99d6f9180db2/pandas/_libs/src/datetime/np_datetime.c#L1087

The word datetime should read timedelta (copy / paste issue). Figured it’s worth fixing while you are touching the file but if you wanted me to fix in a separate issue and PR let me know
Good call. There’s also some indentation in timedeltastruct I’ll get to, could use a double-checking to make sure I don’t screw it up.
# [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/18443?src=pr&el=h1) Report
> Merging [#18443](https://codecov.io/gh/pandas-dev/pandas/pull/18443?src=pr&el=desc) into [master](https://codecov.io/gh/pandas-dev/pandas/commit/fedc503f0d25b04850a931398fc0556d892d69ea?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/18443/graphs/tree.svg?width=650&height=150&src=pr&token=eZ4WkYLtcO)](https://codecov.io/gh/pandas-dev/pandas/pull/18443?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master   #18443      +/-   ##
==========================================
- Coverage   91.35%   91.34%   -0.02%     
==========================================
  Files         163      163              
  Lines       49691    49691              
==========================================
- Hits        45397    45388       -9     
- Misses       4294     4303       +9
```

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

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

------

[Continue to review full report at Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/18443?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/18443?src=pr&el=footer). Last update [fedc503...c08e4ce](https://codecov.io/gh/pandas-dev/pandas/pull/18443?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/18443?src=pr&el=h1) Report
> Merging [#18443](https://codecov.io/gh/pandas-dev/pandas/pull/18443?src=pr&el=desc) into [master](https://codecov.io/gh/pandas-dev/pandas/commit/aaee541b538559f8887881ab23d2734dddd920d3?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/18443/graphs/tree.svg?height=150&src=pr&token=eZ4WkYLtcO&width=650)](https://codecov.io/gh/pandas-dev/pandas/pull/18443?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master   #18443      +/-   ##
==========================================
- Coverage   91.36%   91.31%   -0.05%     
==========================================
  Files         163      163              
  Lines       49700    49700              
==========================================
- Hits        45407    45386      -21     
- Misses       4293     4314      +21
```

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

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

------

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

ping me when complete and I’d be glad to double check
@WillAyd ping.  Should there be an extra indent after `case PANDAS_FR_ns:`?  I'm not that familiar with C syntax.

If we do move the timedelta_struct stuff into cython, let's not bother with the conditioning on meta.base since only one is ever used.
Travis error in test_geopandas

```
>   from fiona.ogrext import Iterator, ItemsIterator, KeysIterator
E   ImportError: libpoppler.so.66: cannot open shared object file: No such file or directory
../../../miniconda3/envs/pandas/lib/python3.6/site-packages/fiona/collection.py:9: ImportError
```
no idea, maybe something is in process of being updated
Referring to the [Google style guide](https://google.github.io/styleguide/cppguide.html#Loops_and_Switch_Statements) again we should have a 2 space indent after the ``case PANDAS_FR_ns:``. 

As I mentioned on a previous comment, I have a feeling that there's either a bug or missing configuration for cpplint to detect those whitespace issues. I'm not overly familiar with the tool so need to research further and advise back.
@WillAyd I'm way out of my element w/r/t C linting, will have to trust you that the 2-spaces is correct.  Are my eye playing tricks on me when the indentation looks inconsistent?  i.e. should I just revert all the indentation edits?

Isn't the switch/case usage here unnecessary anyway?  Or are there plans to implement and use other units?
You are correct that the switch/case usage is at the moment unnecessary, because the method is only being called with nanosecond frequency. The reason I kept it in there was to mirror the implementation that was done for datetime.

As far as the indentation is concerned, that's up to @jreback. I would suggest going forward with your 4 space indentation because it's consistent with the rest of the file. From what I can tell cpplint isn't going to care either way.
> The reason I kept it in there was to mirror the implementation that was done for datetime.

That makes sense.  In the interest of keeping this already-too-wide-ranging PR from expanding scope, I'd like to leave it as is for now, i.e. use 4-space consistently and keep the unnecessary switch/case.  Later I think we should implement the pandas_timedeltastruct and td64_to_tdstruct in tslibs.np_timedeltas directly in cython.  If there isn't a consensus on the latter, achieving it on the former can be good enough for now. 
thanks @jbrockmendel as always, keeping scope narrow is better per PR
I am not ticking off boxes in #17652 but pls do (and link to the issues)