DEPR: Deprecate the convert parameter completely
State: closed

Previously, we weren't issuing a warning if the user happened to pass in the original default of `True`,
which would cause downstream code to break.

Closes #17828

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

Comments:
The new kwarg gets past the original failure but need a more robust comparator.
# [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/17831?src=pr&el=h1) Report
> Merging [#17831](https://codecov.io/gh/pandas-dev/pandas/pull/17831?src=pr&el=desc) into [master](https://codecov.io/gh/pandas-dev/pandas/commit/727ea20bdb72ff4a5b8380d7f613d4aff058e28a?src=pr&el=desc) will **increase** coverage by `<.01%`.
> The diff coverage is `88.88%`.

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

```diff
@@            Coverage Diff             @@
##           master   #17831      +/-   ##
==========================================
+ Coverage   91.22%   91.22%   +<.01%     
==========================================
  Files         163      163              
  Lines       50014    50016       +2     
==========================================
+ Hits        45627    45629       +2     
  Misses       4387     4387
```

| Flag | Coverage Δ | |
|---|---|---|
| #multiple | `89.03% <88.88%> (+0.01%)` | :arrow_up: |
| #single | `40.24% <55.55%> (-0.07%)` | :arrow_down: |

| [Impacted Files](https://codecov.io/gh/pandas-dev/pandas/pull/17831?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pandas/core/generic.py](https://codecov.io/gh/pandas-dev/pandas/pull/17831?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvZ2VuZXJpYy5weQ==) | `92.09% <100%> (ø)` | :arrow_up: |
| [pandas/core/sparse/series.py](https://codecov.io/gh/pandas-dev/pandas/pull/17831?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvc3BhcnNlL3Nlcmllcy5weQ==) | `95.28% <80%> (+0.01%)` | :arrow_up: |
| [pandas/io/gbq.py](https://codecov.io/gh/pandas-dev/pandas/pull/17831?src=pr&el=tree#diff-cGFuZGFzL2lvL2dicS5weQ==) | `25% <0%> (-58.34%)` | :arrow_down: |
| [pandas/core/indexing.py](https://codecov.io/gh/pandas-dev/pandas/pull/17831?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvaW5kZXhpbmcucHk=) | `92.82% <0%> (-0.19%)` | :arrow_down: |
| [pandas/core/frame.py](https://codecov.io/gh/pandas-dev/pandas/pull/17831?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvZnJhbWUucHk=) | `97.77% <0%> (-0.1%)` | :arrow_down: |
| [pandas/core/indexes/datetimes.py](https://codecov.io/gh/pandas-dev/pandas/pull/17831?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvaW5kZXhlcy9kYXRldGltZXMucHk=) | `95.48% <0%> (-0.1%)` | :arrow_down: |
| [pandas/plotting/\_converter.py](https://codecov.io/gh/pandas-dev/pandas/pull/17831?src=pr&el=tree#diff-cGFuZGFzL3Bsb3R0aW5nL19jb252ZXJ0ZXIucHk=) | `65.2% <0%> (+1.81%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/17831?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/17831?src=pr&el=footer). Last update [727ea20...762fd6e](https://codecov.io/gh/pandas-dev/pandas/pull/17831?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/17831?src=pr&el=h1) Report
> Merging [#17831](https://codecov.io/gh/pandas-dev/pandas/pull/17831?src=pr&el=desc) into [master](https://codecov.io/gh/pandas-dev/pandas/commit/7e159aeec1d5d2e79311ca688c0e594f0fd5f709?src=pr&el=desc) will **decrease** coverage by `0.04%`.
> The diff coverage is `80%`.

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

```diff
@@            Coverage Diff             @@
##           master   #17831      +/-   ##
==========================================
- Coverage   91.25%    91.2%   -0.05%     
==========================================
  Files         163      163              
  Lines       50038    50040       +2     
==========================================
- Hits        45661    45638      -23     
- Misses       4377     4402      +25
```

| Flag | Coverage Δ | |
|---|---|---|
| #multiple | `89.01% <80%> (-0.04%)` | :arrow_down: |
| #single | `40.26% <20%> (-0.07%)` | :arrow_down: |

| [Impacted Files](https://codecov.io/gh/pandas-dev/pandas/pull/17831?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pandas/core/generic.py](https://codecov.io/gh/pandas-dev/pandas/pull/17831?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvZ2VuZXJpYy5weQ==) | `92.2% <ø> (ø)` | :arrow_up: |
| [pandas/core/sparse/series.py](https://codecov.io/gh/pandas-dev/pandas/pull/17831?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvc3BhcnNlL3Nlcmllcy5weQ==) | `95.28% <80%> (+0.01%)` | :arrow_up: |
| [pandas/io/gbq.py](https://codecov.io/gh/pandas-dev/pandas/pull/17831?src=pr&el=tree#diff-cGFuZGFzL2lvL2dicS5weQ==) | `25% <0%> (-58.34%)` | :arrow_down: |
| [pandas/plotting/\_converter.py](https://codecov.io/gh/pandas-dev/pandas/pull/17831?src=pr&el=tree#diff-cGFuZGFzL3Bsb3R0aW5nL19jb252ZXJ0ZXIucHk=) | `63.38% <0%> (-1.82%)` | :arrow_down: |
| [pandas/compat/numpy/function.py](https://codecov.io/gh/pandas-dev/pandas/pull/17831?src=pr&el=tree#diff-cGFuZGFzL2NvbXBhdC9udW1weS9mdW5jdGlvbi5weQ==) | `92.12% <0%> (-1.22%)` | :arrow_down: |
| [pandas/core/indexing.py](https://codecov.io/gh/pandas-dev/pandas/pull/17831?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvaW5kZXhpbmcucHk=) | `92.82% <0%> (-0.19%)` | :arrow_down: |
| [pandas/core/frame.py](https://codecov.io/gh/pandas-dev/pandas/pull/17831?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvZnJhbWUucHk=) | `97.75% <0%> (-0.1%)` | :arrow_down: |

------

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

Everything is passing.  PTAL.
Can I please unsubscribe

On Tue, Oct 10, 2017 at 7:11 AM gfyoung <notifications@github.com> wrote:

> Previously, we weren't issuing a warning if the user happened to pass in
> the original default of True,
> which would cause downstream code to break.
>
> Closes #17828 <https://github.com/pandas-dev/pandas/issues/17828>
> ------------------------------
> You can view, comment on, or merge this pull request online at:
>
>   https://github.com/pandas-dev/pandas/pull/17831
> Commit Summary
>
>    - DEPR: Deprecate the convert parameter completely
>
> File Changes
>
>    - *M* pandas/compat/numpy/function.py
>    <https://github.com/pandas-dev/pandas/pull/17831/files#diff-0> (6)
>    - *M* pandas/core/generic.py
>    <https://github.com/pandas-dev/pandas/pull/17831/files#diff-1> (13)
>    - *M* pandas/core/sparse/series.py
>    <https://github.com/pandas-dev/pandas/pull/17831/files#diff-2> (8)
>    - *M* pandas/tests/frame/test_axis_select_reindex.py
>    <https://github.com/pandas-dev/pandas/pull/17831/files#diff-3> (4)
>    - *M* pandas/tests/sparse/test_series.py
>    <https://github.com/pandas-dev/pandas/pull/17831/files#diff-4> (14)
>
> Patch Links:
>
>    - https://github.com/pandas-dev/pandas/pull/17831.patch
>    - https://github.com/pandas-dev/pandas/pull/17831.diff
>
> —
> You are receiving this because you are subscribed to this thread.
> Reply to this email directly, view it on GitHub
> <https://github.com/pandas-dev/pandas/pull/17831>, or mute the thread
> <https://github.com/notifications/unsubscribe-auth/AAJfa3Nd7QUQ4eegHECsoBwQ54cWa-D2ks5sqvxrgaJpZM4PzYie>
> .
>
-- 
Greg Gurevich
greggurevich@gmail.com
cell: 1-917-504-7105

> Can I please unsubscribe

@bkmrkr If you scroll above, there is a button 'Unwatch' (or watch), if you click on the dropdown, you can choose which issue to follow.
Thoughts @jreback? This is blocking the RC now.

@gfyoung could you explain in a bit more detail why making `convert=None` the default breaks compat w/ older NumPy? I would think that something like

```python
def take(self, indices, axis=0, convert=None, is_copy=True, **kwargs):
    if convert is None:
        convert = True
    else:
        warnings.warn(...)
```

would work fine. Not a huge deal, just a little strange to use -1 instead of None I think.
@TomAugspurger : I've tried various logic (including that one), and older versions of `numpy` don't react well when you try to call `np.take`.  The reason is that `take=None` is getting confused with older `numpy`'s positional argument function calls internally.  Trying to accommodate `numpy` results in me breaking compatibility with perfectly reasonable function calls to `.take()` in `pandas`.

If we were to drop support for `numpy` < `1.12`, this wouldn't be an issue 😄 
@gfyoung I think let's keep the signature with ``convert=None``. If we break ``np.take(...)`` of a pandas expression, too bad. just fix the test (and you can put a note). putting in a -1 is really odd here.
> If we were to drop support for numpy < 1.12, this wouldn't be an issue

not goning to happen :> (actually you can leave the test for >= 1.12 for this as it will work with ``convert=None``
> I think let's keep the signature with convert=None. If we break np.take(...) of a pandas expression, too bad.

Okay, will do.

> not goning to happen :>

Oh, that's too bad.  I thought there was a good chance we would do that :smile: 
Finally, all is green again.  PTAL.
lgtm module a small doc comment..