Skip to content

DEP: Make np.delete on out-of-bounds indices an error#15804

Merged
seberg merged 1 commit intonumpy:masterfrom
eric-wieser:expire-delete-out-of-bounds
Mar 23, 2020
Merged

DEP: Make np.delete on out-of-bounds indices an error#15804
seberg merged 1 commit intonumpy:masterfrom
eric-wieser:expire-delete-out-of-bounds

Conversation

@eric-wieser
Copy link
Member

@eric-wieser eric-wieser commented Mar 22, 2020

Note that this only affects lists of indices.

>>> a = np.arange(3)

Before:

>>> np.delete(a, 100)
IndexError
>>> np.delete(a, [100])
DeprecationWarning
array([0, 1, 2])

>>> np.delete(a, -1)
array([0, 1])
>>> np.delete(a, [-1])
FutureWarning
array([0, 1, 2])

After:

>>> np.delete(a, 100)
IndexError
>>> np.delete(a, [100])
IndexError

>>> np.delete(a, -1)
array([0, 1])
>>> np.delete(a, [-1])
array([0, 1])

Similar to gh-15802

Note that this only affects lists of indices.

```python
>>> a = np.arange(3)
````
Before:
```python
>>> np.delete(a, 100)
IndexError
>>> np.delete(a, [100])
DeprecationWarning
array([0, 1, 2])

>>> np.delete(a, -1)
array([0, 1])
>>> np.delete(a, [-1])
FutureWarning
array([0, 1, 2])
```

After:
```python
>>> np.delete(a, 100)
IndexError
>>> np.delete(a, [100])
IndexError

>>> np.delete(a, -1)
array([0, 1])
>>> np.delete(a, [-1])
array([0, 1])
```
@seberg
Copy link
Member

seberg commented Mar 23, 2020

Thanks Eric, LGTM.

@seberg seberg merged commit d5010b1 into numpy:master Mar 23, 2020
Comment on lines 831 to 833
with warnings.catch_warnings(record=True) as w:
warnings.filterwarnings('always', category=FutureWarning)
self._check_inverse_of_slicing([0, -1, 2, 2])
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm pretty sure I made a mistake in this patch, I'd expect this to produce one fewer warning, yet it appears to pass.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmm, strange, then why did this not fail? There are two delete calls in there, but the FutureWarning that is being detected here must not be the one that you deleted. Maybe something completely unrelated?

Copy link
Member

@seberg seberg Mar 23, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wait, below the boolean index still gives a futurewarning right? And there are two calls of delete. So I guess it should have checked the warnings 4 times, or reset in between the two checks simply. Seems fair enough, the boolean fixup just needs to delete it all.

EDIT: Nvm. I see you already opened a PR to that effect.

ueshin added a commit to databricks/koalas that referenced this pull request Jul 17, 2020
Since numpy 1.19, `np.delete` came to fail on out-of-bounds indices (numpy/numpy#15804) and pandas will as well since it depends on numpy.
rising-star92 added a commit to rising-star92/databricks-koalas that referenced this pull request Jan 27, 2023
Since numpy 1.19, `np.delete` came to fail on out-of-bounds indices (numpy/numpy#15804) and pandas will as well since it depends on numpy.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants