Skip to content

BUG: Make ediff1d kwarg casting consistent#14981

Merged
mattip merged 1 commit intonumpy:masterfrom
seberg:issue-13103
Dec 11, 2019
Merged

BUG: Make ediff1d kwarg casting consistent#14981
mattip merged 1 commit intonumpy:masterfrom
seberg:issue-13103

Conversation

@seberg
Copy link
Member

@seberg seberg commented Nov 25, 2019

This reverts commit c088383 and
uses the same kind casting rule for the additional keyword arguments
to_end and to_begin. This results in slightly more leniant
behaviour for integers (which can now have overflows that are
hidden), but fixes an issue with the handling of NaN.
Generally, this behaviour seems more conistent with what NumPy does
elsewhere. The Overflow issue exists similar in many other places
and should be solved by integer overflow warning machinery while
the actual cast takes place.

Closes gh-13103

@mattip
Copy link
Member

mattip commented Nov 25, 2019

One minor comment nit, otherwise LGTM.

This reverts commit c088383 and
uses the same kind casting rule for the additional keyword arguments
``to_end`` and ``to_begin``. This results in slightly more leniant
behaviour for integers (which can now have overflows that are
hidden), but fixes an issue with the handling of NaN.
Generally, this behaviour seems more conistent with what NumPy does
elsewhere. The Overflow issue exists similar in many other places
and should be solved by integer overflow warning machinery while
the actual cast takes place.

Closes numpygh-13103
@mattip
Copy link
Member

mattip commented Dec 4, 2019

I wonder if the change in behaviour will affect users in practice. Should we put this in with the caveat that we may need to revert it, or is there a better way to try it out?

@mattip mattip merged commit 5856c48 into numpy:master Dec 11, 2019
@mattip
Copy link
Member

mattip commented Dec 11, 2019

Thanks @seberg

@stuartarchibald
Copy link
Contributor

Thanks for sorting this, much appreciated.

@charris
Copy link
Member

charris commented Dec 13, 2019

Worth a backport, or can we just treat it as a bugfix?

@seberg
Copy link
Member Author

seberg commented Dec 13, 2019

Hmmm, good question. It fixes an annoying bug in a sense, but it is also more strict, so not 100% foolproof backport maybe.

@seberg seberg added the triage review Issue/PR to be discussed at the next triage meeting label Dec 13, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

00 - Bug component: numpy._core triage review Issue/PR to be discussed at the next triage meeting

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ediff1d with np.nan in to_begin/to_end, behaviour and error messages

4 participants