Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BUG: bool(dtype) is True #8279

Merged
merged 1 commit into from
Jan 23, 2017
Merged

BUG: bool(dtype) is True #8279

merged 1 commit into from
Jan 23, 2017

Conversation

jcrist
Copy link
Contributor

@jcrist jcrist commented Nov 15, 2016

Previously bool(dtype(...)) would fallback to the default
implementation of __nonzero__, which checks if len(object) > 0.
Since dtype objects implement __len__ as the number of record
fields, bool of scalar dtypes like bool(dtype('i8')) would evaluate
as False. This fixes that by implementing __nonzero__ to always
return True.

Fixes #6294.

return 1;
}

static PyNumberMethods descr_as_number = {
Copy link
Member

@charris charris Nov 16, 2016

Choose a reason for hiding this comment

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

Not Python 3 compatible, see doc/Py3K.rst.txt or grep for examples. Also, Python tends to 0 rather than NULL.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, fixed.

@@ -3600,6 +3600,31 @@ arraydescr_richcompare(PyArray_Descr *self, PyObject *other, int cmp_op)
return result;
}

static Py_ssize_t
descr_nonzero(PyObject *self)
Copy link
Member

Choose a reason for hiding this comment

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

Wrong return type. The CPython typedef is

typedef int (*inquiry)(PyObject *);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks. Should be good now.

@seberg
Copy link
Member

seberg commented Jan 5, 2017

Seems a small thing really, should we just put this in? The only thing I could find against is whether V0 is True (since it has itemsize 0), but is a rather theoretical thing as well.

@jcrist maybe you can add a short mention to the 1.13 release notes (or am I overdoing release notes ;)).

@charris
Copy link
Member

charris commented Jan 5, 2017

@seberg I don't have a problem with this, go ahead. A release note would be good, someone may actually be using current behavior to check for fields. Hmm, what is the recommended way to do that?

@seberg
Copy link
Member

seberg commented Jan 6, 2017

Hmmm, true, I guess you can check whether dtype.fields is None (or .names), or actually go back to checking len(dtype), not sure what is best. A bit tricky to decide if any dtype should evaluate to False, hmmm.

@charris
Copy link
Member

charris commented Jan 6, 2017

@seberg Yep, a grep turns up if dtype.fields is None and if dtype fields. Oh, oh, two ways of doing something in Python 8-)

@seberg
Copy link
Member

seberg commented Jan 6, 2017 via email

@charris
Copy link
Member

charris commented Jan 21, 2017

@jcrist Could you add something to the doc/release/1.13.0-notes.rst.

@charris charris added the 56 - Needs Release Note. Needs an entry in doc/release/upcoming_changes label Jan 21, 2017
Previously `bool(dtype(...))` would fallback to the default
implementation of `__nonzero__`, which checks if `len(object) > 0`.
Since `dtype` objects implement `__len__` as the number of record
fields, `bool` of scalar dtypes like `bool(dtype('i8'))` would evaluate
as `False`. This fixes that by implementing `__nonzero__` to always
return True.

Fixes numpy#6294.
@jcrist
Copy link
Contributor Author

jcrist commented Jan 23, 2017

Apologies for the delay here. I've added a release note.

@charris charris merged commit 81d176e into numpy:master Jan 23, 2017
@charris
Copy link
Member

charris commented Jan 23, 2017

@jcrist Thanks.

@jcrist jcrist deleted the dtypes-are-true branch January 23, 2017 19:27
jreback added a commit to jreback/pandas that referenced this pull request Jan 23, 2017
jreback added a commit to jreback/pandas that referenced this pull request Jan 24, 2017
jreback added a commit to jreback/pandas that referenced this pull request Jan 24, 2017
jreback added a commit to jreback/pandas that referenced this pull request Jan 24, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
00 - Bug 56 - Needs Release Note. Needs an entry in doc/release/upcoming_changes component: numpy._core
Projects
None yet
Development

Successfully merging this pull request may close these issues.

odd boolean result for np.dtype
3 participants