10000 ENH: Implement ndarray.__format__ for 0d arrays by eric-wieser · Pull Request #9883 · numpy/numpy · GitHub
[go: up one dir, main page]
Skip to content

ENH: Implement ndarray.__format__ for 0d arrays #9883

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Oct 18, 2017
Merged

ENH: Implement ndarray.__format__ for 0d arrays #9883

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
53a022b
ENH: Implement ndarray.__format__ for 0d arrays
eric-wieser Oct 18, 2017
86a7ac6
STY: Add blank line after variable declarations.
charris Oct 18, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions numpy/core/src/multiarray/methods.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#include "item_selection.h"
#include "conversion_utils.h"
#include "shape.h"
#include "strfuncs.h"

#include "methods.h"
#include "alloc.h"
Expand Down Expand Up @@ -2533,6 +2534,10 @@ NPY_NO_EXPORT PyMethodDef array_methods[] = {
(PyCFunction) array_complex,
METH_VARARGS, NULL},

{"__format__",
(PyCFunction) array_format,
METH_VARARGS, NULL},

#ifndef NPY_PY3K
/*
* While we could put these in `tp_sequence`, its' easier to define them
Expand Down
28 changes: 28 additions & 0 deletions numpy/core/src/multiarray/strfuncs.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -197,3 +197,31 @@ array_str(PyArrayObject *self)
}
return s;
}

NPY_NO_EXPORT PyObject *
array_format(PyArrayObject *self, PyObject *args)
{
PyObject *format;
if (!PyArg_ParseTuple(args, "O:__format__", &format))
return NULL;

/* 0d arrays - forward to the scalar type */
if (PyArray_NDIM(self) == 0) {
PyObject *item = PyArray_ToScalar(PyArray_DATA(self), self);
PyObject *res;

if (item == NULL) {
return NULL;
}
res = PyObject_Format(item, format);
Py_DECREF(item);
return res;
}
/* Everything else - use the builtin */
Copy link
Member

Choose a reason for hiding this comment

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

What does the builtin do here?

Copy link
Member Author
@eric-wieser eric-wieser Oct 18, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

It keeps the arbitrary and python-dependent behavior that we had before this patch.

Most importantly it handles '{}'.format in the typical way. On python 3, it throws an exception if a format string is given, while on python 2 it evaluates format(str(arr), 'theformatspec').

else {
return PyObject_CallMethod(
(PyObject *)&PyBaseObject_Type, "__format__", "OO",
(PyObject *)self, format
);
}
}
3 changes: 3 additions & 0 deletions numpy/core/src/multiarray/strfuncs.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,4 +10,7 @@ array_repr(PyArrayObject *self);
NPY_NO_EXPORT PyObject *
array_str(PyArrayObject *self);

NPY_NO_EXPORT PyObject *
array_format(PyArrayObject *self, PyObject *args);

#endif
31 changes: 31 additions & 0 deletions numpy/core/tests/test_multiarray.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7013,6 +7013,37 @@ def test_null_inside_ustring_array_is_truthy(self):
assert_(a)


class TestFormat(object):

def test_0d(self):
a = np.array(np.pi)
assert_equal('{:0.3g}'.format(a), '3.14')
Copy link
Member

Choose a reason for hiding this comment

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

Is assert_equal the best way to compare strings? Maybe just assert_(str1 == str2) would be a bit cleaner.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, because then it shows the two values if they differ.

Copy link
Member

Choose a reason for hiding this comment

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

OK.

Copy link
Contributor

Choose a reason for hiding this comment

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

In pytest you can just assert str1 == str2 and you will get values if there is a failure.


def test_failure():
    a = 'apple'
    b = 'banana'
    assert a == b

Output is

================================== FAILURES ===================================
________________________________ test_failure _________________________________

    def test_failure():
        a = 'apple'
        b = 'banana'
>       assert a == b
E       AssertionError: assert 'apple' == 'banana'
E         - apple
E         + banana

test_util.py:20: AssertionError
========================== 1 failed in 0.24 seconds ===========================

Copy link
Member Author

Choose a reason for hiding this comment

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

For consistency, we should probably use the same form we use in most of the test suite, which gives:

>>> np.testing.assert_equal('a', 'b')
AssertionError: 
Items are not equal:
 ACTUAL: 'a'
 DESIRED: 'b'

AFAIK, the switch to pytest was recent, and we were previously using nose

Copy link
Member

Choose a reason for hiding this comment

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

We are still using nose, th 6DB6 e pytest conversion is still in progress and I don't think I will get back to it until 1.15.

assert_equal('{:0.3g}'.format(a[()]), '3.14')

def test_1d_no_format(self):
a = np.array([np.pi])
assert_equal('{}'.format(a), str(a))

def test_1d_format(self):
# until gh-5543, ensure that the behaviour matches what it used to be
a = np.array([np.pi])

def ret_and_exc(f, *args, **kwargs):
try:
return f(*args, **kwargs), None
except Exception as e:
# exceptions don't compare equal, so return type and args
# which do
return None, (type(e), e.args)

# Could switch on python version here, but all we care about is
# that the behaviour hasn't changed
assert_equal(
ret_and_exc(object.__format__, a, '30'),
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 suppose we should silence warnings in ret_and_exc, or return a third tuple item which is the warning

Copy link
Member

Choose a reason for hiding this comment

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

Maybe just figure out what the result should be and use that? Eventually object.__format__ is going to fail with non-empty argument.

Copy link
Member Author

Choose a reason for hiding this comment

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

Right, and I think that ndarray.__format__ should start failing at the same time. Maybe we should just make ndarray.__format__ fail with an empty argument right now...

Copy link
Member

Choose a reason for hiding this comment

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

Looks like it went away in Python 3.4.

* :meth:`object.__format__` no longer accepts non-empty format strings, it now
  raises a :exc:`TypeError` instead.  Using a non-empty string has been
  deprecated since Python 3.2.  This change has been made to prevent a
  situation where previously working (but incorrect) code would start failing
  if an object gained a __format__ method, which means that your code may now
  raise a :exc:`TypeError` if you are using an ``'s'`` format code with objects
  that do not have a __format__ method that handles it.  See :issue:`7994` for
  background.

Should ours work if an empty string is passed?

Copy link
Member

Choose a reason for hiding this comment

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

I fixed this by making the test Python version dependent.

ret_and_exc('{:30}'.format, a)
)


class TestCTypes(object):

def test_ctypes_is_available(self):
Expand Down
0