ENH: Added support for arrays with `dtype=object` to `np.isinf`, `np.isnan`, `np.isfinite` #10820

madphysicist · 2018-03-29T06:09:08Z

The loops check if the object has a __float__ or __complex__ method, then handle the object as usual. Preference is given to __float__ in all cases. This is a preliminary step to make sure I understand the basics of how to add to a ufunc, so please grade harshly! The next step will be adding datetime and timedelta support to np.isfinite, in preparation for adding support to np.histogram.

Two questions arise:

Why are the integer loops for isinite, isinf, isnan not optimized to always return True regardless of the input data?
a. Am I correct to assume that the values are actually cast to floating point before running through the normal loop?
b. Is there any desire to change this? Or conversely, any motivation not to?
Why are isneginf and isposinf not ufuncs like isinf? There does not appear to be any major issue except a minor break in backwards compatibility where the second parameter would be renamed from y to out.

madphysicist · 2018-03-29T06:23:31Z

numpy/core/code_generators/generate_umath.py

          docstrings.get('numpy.core.umath.isfinite'),
          None,
-          TD(inexact, out='?'),
+          TD(noint, out='?'),


What happens if I have multiple overlapping TD calls? e.g., if I hadn't deleted the line TD(inexact, out='?') here.

madphysicist · 2018-03-29T06:24:24Z

numpy/core/tests/test_umath.py

        assert_raises(TypeError, test_nan)

+
+class _CustomFloat(object):


If there is already an implementation of mocks like these somewhere, I would be happy to get rid of them.

Can you use unittest.mock?

madphysicist · 2018-03-29T06:26:40Z

numpy/core/src/umath/loops.c.src

+    int v;
+
+    UNARY_LOOP {
+        PyObject *in1 = *(PyObject **)ip1;


I am almost 100% sure that I don't need to Py_INCREF(in1) here or anywhere else, but I feel like someone should verify that since I do pass it to PyFloat_AsDouble and PyComplex_AsCComplex down the line.

madphysicist · 2018-03-29T06:28:08Z

numpy/core/src/umath/loops.c.src

+            if (cplx.real == -1.0 && cplx.imag == 0.0 && PyErr_Occurred()) {
+                PyErr_Format(PyExc_TypeError, "must be real or complex number, not %s",
+                             Py_TYPE(in1)->tp_name);
+                break;


Does a corresponding Py_DECREF(in1) need to go here? (see above)

madphysicist · 2018-03-29T06:28:27Z

numpy/core/src/umath/loops.c.src

+        else {
+            v = @func@(dbl) != 0;
+        }
+        *((npy_bool *)op1) = v;


Does a corresponding Py_DECREF(in1) need to go here? (see above)

eric-wieser · 2018-03-29T07:32:04Z

I think this would be a little cleaner via funcs.inc.src, in a similar way to how npy_ObjectLogicalNot is implemented. You should be able to just move most of your implementation to that file

Nothing seemed worth adding to the docs of the functions themselves.

madphysicist · 2018-03-29T20:07:40Z

@eric-wieser. I have moved the implementation to funcs.inc.src. This includes changing the output dtype to np.object for cases when the input is np.object. I've also added comments to the docs for that.

I added support for integers (objects implementing __int__/__long__) as well. If you have a better idea about where to put the sub-function that checks for integer types, please let me know.

I've added a couple more comments to my code about concerns that I have with the correctness.

eric-wieser · 2018-03-29T20:08:22Z

doc/release/1.14.0-notes.rst

 .. _c-api: https://github.com/numpy/numpy/blob/master/doc/source/reference/c-api.array.rst
 .. _how-to-extend: https://github.com/numpy/numpy/blob/master/doc/source/user/c-info.how-to-extend.rst
+Support for ``dtype=object`` in ``isinf``, ``isnan``, ``isfinite``
+Support for ``dtype=object`` in ``isnan``, ``isinf``, ``isfinite``


Rebase mistake

eric-wieser · 2018-03-29T20:10:29Z

Note this duplicates #6320 - you might want to compare to the approach used there

madphysicist · 2018-03-29T20:10:27Z

numpy/core/code_generators/generate_umath.py

    Ufunc(1, 1, None,
          docstrings.get('numpy.core.umath.isnan'),
          None,
          TD(inexact, out='?'),


What happens if I have multiple overlapping TD calls? e.g., if this line read TD(noint, out='?'), making conflicting protoypes for dtype=object?

madphysicist · 2018-03-29T20:12:33Z

numpy/core/src/umath/funcs.inc.src

+    if(i1 == NULL) {
+        return NULL;
+    }
+    else {


I am almost 100% sure that I don't need to Py_INCREF(i1) here or anywhere else, but I feel like someone should verify that since I do pass it to PyFloat_AsDouble, PyComplex_AsCComplex, etc down the line.

Just because all my tests pass does not mean that the refcounting is done correctly here.

madphysicist · 2018-03-29T20:14:40Z

numpy/core/src/umath/funcs.inc.src

    return PyNumber_Absolute(tmp);
 }

+/* Utility used to check if a number is an integer or has an __int__/__long__ method */


An alternative implementation is suggested by https://stackoverflow.com/a/49562820/2988730. However, using PyNumber_Long/PyNumber_Int will allow converstion of strings and objects with just __trunc__ but no __float__, which I don't think we want.

madphysicist · 2018-03-29T20:49:36Z

@eric-wieser I agree that this should defer to #6320. I especially like the part where the result is always np.bool. This PR has two valuable things though: it calls __float__ and __complex__ as necessary, and it checks for integers. Basically, it treats python types more like you would expect numpy to treat them based on how it treats its own types.

I will leave this around for a bit longer to see if anyone can give me more pointers. In the meantime, I will go work on adding datetimes to np.isfinite and eventually to np.histogram.

eric-wieser · 2018-03-29T20:51:29Z

In the meantime, I will go work on adding datetimes to np.isfinite and eventually to np.histogram.

Sounds good to me. Maybe add loops for the integer types too. I think there's a dead PR for that somewhere too.

madphysicist · 2018-03-29T21:05:54Z

@eric-wieser Let me know if you find it. That was one of the questions I had at the top. At least for the three functions that I have here, you don't even need a loop, just a boolean array of all ones. Is that something that ufuncs can easily support?

eric-wieser · 2018-03-29T21:24:57Z

You just write a loop that ignores its inputs and writes ones to the output.

eric-wieser · 2019-02-19T06:37:00Z

Let me know if you find it.

Still looking, but found a related one for bools: #12988

mattip · 2019-11-03T20:03:03Z

@madphysicist would you like to continue with this? The release note should be rewritten as a fragment in doc/release/upcoming_changes and the conflicts resolved to start to move forward.

madphysicist · 2019-11-06T23:25:04Z

@mattip. Eventually. I'm going to have to re-familiarize myself with what I did here first.

mattip · 2024-02-07T18:37:55Z

Closing. This has been around for quite a while and has not moved very far. Please reopen if you want to pursue the idea.

madphysicist commented Mar 29, 2018

View reviewed changes

charris added 01 - Enhancement component: numpy._core labels Mar 29, 2018

madphysicist added 7 commits March 29, 2018 16:02

MAINT: Minor fixups to ufunc generation

a2f5a00

ENH: Added dtype=object to isinf, isnan, and isfinite

988d55f

TST: Added tests for dtype=object in isinf, isnan, and isfinite

9311d09

DOC: Added changes to release notes

68aded0

Nothing seemed worth adding to the docs of the functions themselves.

MAINT: Fixed line lengths

c49b332

MAINT: Moved implementation of isnan,isinf,isfinite object support

4d7b194

DOC: Documented updates to isnan,isinf,isfinite

30928b5

madphysicist force-pushed the ufunc-finite-object branch from 1787c1a to 30928b5 Compare March 29, 2018 20:07

eric-wieser reviewed Mar 29, 2018

View reviewed changes

madphysicist commented Mar 29, 2018

View reviewed changes

DOC: Corrected release notes

277c27c

eric-wieser mentioned this pull request Jun 22, 2018

np.exp raises AttributeError when called with large integer #11407

Open

eric-wieser mentioned this pull request Oct 30, 2019

ENH: Add object loops to isnan, isinf, and isfinite #14802

Closed

Base automatically changed from master to main March 4, 2021 02:04

seberg added 54 - Needs decision 57 - Close? Issues which may be closable unless discussion continued labels Jul 8, 2021

mattip closed this Feb 7, 2024

		assert_raises(TypeError, test_nan)


		class _CustomFloat(object):

Uh oh!

ENH: Added support for arrays with dtype=object to np.isinf, np.isnan, np.isfinite #10820

ENH: Added support for arrays with dtype=object to np.isinf, np.isnan, np.isfinite #10820

Uh oh!

Conversation

madphysicist commented Mar 29, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

eric-wieser commented Mar 29, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

madphysicist commented Mar 29, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

eric-wieser commented Mar 29, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

madphysicist commented Mar 29, 2018

Uh oh!

eric-wieser commented Mar 29, 2018

Uh oh!

madphysicist commented Mar 29, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

eric-wieser commented Mar 29, 2018

Uh oh!

eric-wieser commented Feb 19, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mattip commented Nov 3, 2019

Uh oh!

madphysicist commented Nov 6, 2019

Uh oh!

mattip commented Feb 7, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

ENH: Added support for arrays with `dtype=object` to `np.isinf`, `np.isnan`, `np.isfinite` #10820

ENH: Added support for arrays with `dtype=object` to `np.isinf`, `np.isnan`, `np.isfinite` #10820

madphysicist commented Mar 29, 2018 •

edited

Loading

eric-wieser commented Mar 29, 2018 •

edited

Loading

madphysicist commented Mar 29, 2018 •

edited

Loading

eric-wieser commented Feb 19, 2019 •

edited

Loading