DEPR: convert_datetime64 parameter in to_records()
State: closed

- [x] closes #18160
- [x] tests added / passed
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [x] whatsnew entry

As noted in the original issue, the underlying NumPy bug seems to be fixed so this parameter may no longer be needed. 

I also changed the default value for ``convert_datetime64`` from ``True`` to ``False`` and only give a ``FutureWarning`` when ``convert_datetime64=True`` is passed but I'm not sure if this is more appropriate than leaving ``True`` as the default and always giving a ``FutureWarning``.

Labels:
name: Dtype Conversions
description: Unexpected or buggy dtype conversions
name: Deprecate
description: Functionality to remove in pandas

Comments:
@reidy-p : I'm a little surprised to see that you didn't have to change any tests with this deprecation, as it is also an API change.  If this passes, it means coverage for this function is not that great.

Perhaps we should take the opportunity to write a few more tests...
@gfyoung yeah I was surprised too. I'll take a look at the existing tests in more detail and see if I can add some more.
# [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/18902?src=pr&el=h1) Report
> Merging [#18902](https://codecov.io/gh/pandas-dev/pandas/pull/18902?src=pr&el=desc) into [master](https://codecov.io/gh/pandas-dev/pandas/commit/3885cedb884a8b22e0875d9ffbc8d28123d82b48?src=pr&el=desc) will **decrease** coverage by `0.02%`.
> The diff coverage is `100%`.

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

```diff
@@            Coverage Diff             @@
##           master   #18902      +/-   ##
==========================================
- Coverage   91.84%   91.81%   -0.03%     
==========================================
  Files         153      153              
  Lines       49275    49277       +2     
==========================================
- Hits        45255    45245      -10     
- Misses       4020     4032      +12
```

| Flag | Coverage Δ | |
|---|---|---|
| #multiple | `90.2% <100%> (-0.03%)` | :arrow_down: |
| #single | `41.91% <33.33%> (-0.01%)` | :arrow_down: |

| [Impacted Files](https://codecov.io/gh/pandas-dev/pandas/pull/18902?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pandas/core/frame.py](https://codecov.io/gh/pandas-dev/pandas/pull/18902/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvZnJhbWUucHk=) | `97.16% <100%> (ø)` | :arrow_up: |
| [pandas/plotting/\_converter.py](https://codecov.io/gh/pandas-dev/pandas/pull/18902/diff?src=pr&el=tree#diff-cGFuZGFzL3Bsb3R0aW5nL19jb252ZXJ0ZXIucHk=) | `65.07% <0%> (-1.74%)` | :arrow_down: |

------

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

can you rebase
(moving this to the main thread)

Which option is best depends on what the end goal here is. 
Do we actually want to remove the option altogether in the future? In that case, we always need to warn if the keyword is manually specified, so we need a default of None to be able to catch this (which can then mean an actual default of True or False, which one of the two is independent from having None as the default)

Given the deprecation warning, it seems we actually want to remove it. So I think we need a default of None (and then best give a different warning in case True or False was specified).

But having a default of None, does not mean that if it is None, it should mean `convert_datetime64=True` as you indicate in your comment above. We can still actually change the behaviour at the same time if we want (the deprecation warning is currently for the keyword, not for the actual behaviour change I think?)

We could also delay the actual change in behaviour and first warn that it will change in the future (your option 1). 
But given this is such a corner case and almost never the desired behaviour, I am not sure it is needed in this case.
if you can update @reidy-p 
@jreback I think this is ready but would be good if you could double-check the tests to make sure they make sense.
lgtm. thanks @reidy-p