Skip to content

ENH: add timedelta modulus operator support (mm) #12120

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

Merged
merged 1 commit into from
Oct 30, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 6 additions & 0 deletions doc/release/1.16.0-notes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,12 @@ New Features
New keyword ``max_rows`` in `numpy.loadtxt` sets the maximum rows of the
content to be read after ``skiprows``, as in `numpy.genfromtxt`.

modulus operator support added for ``np.timedelta64`` operands
--------------------------------------------------------------
The modulus (remainder) operator is now supported for two operands
of type ``np.timedelta64``. The operands may have different units
and the return value will match the type of the operands.


Improvements
============
Expand Down
3 changes: 3 additions & 0 deletions doc/source/reference/arrays.datetime.rst
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,9 @@ simple datetime calculations.
>>> np.timedelta64(1,'W') / np.timedelta64(1,'D')
7.0

>>> np.timedelta64(1,'W') % np.timedelta64(10,'D')
numpy.timedelta64(7,'D')

There are two Timedelta units ('Y', years and 'M', months) which are treated
specially, because how much time they represent changes depending
on when they are used. While a timedelta day unit is equivalent to
Expand Down
3 changes: 2 additions & 1 deletion numpy/core/code_generators/generate_umath.py
Original file line number Diff line number Diff line change
Expand Up @@ -791,8 +791,9 @@ def english_upper(s):
'remainder':
Ufunc(2, 1, None,
docstrings.get('numpy.core.umath.remainder'),
None,
'PyUFunc_RemainderTypeResolver',
TD(intflt),
[TypeDescription('m', FullTypeDescr, 'mm', 'm')],
TD(O, f='PyNumber_Remainder'),
),
'divmod':
Expand Down
28 changes: 28 additions & 0 deletions numpy/core/src/umath/loops.c.src
Original file line number Diff line number Diff line change
Expand Up @@ -1591,6 +1591,34 @@ TIMEDELTA_mm_d_divide(char **args, npy_intp *dimensions, npy_intp *steps, void *
}
}

NPY_NO_EXPORT void
TIMEDELTA_mm_m_remainder(char **args, npy_intp *dimensions, npy_intp *steps, void *NPY_UNUSED(func))
Copy link
Member

Choose a reason for hiding this comment

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

This function looks correct, thanks

{
BINARY_LOOP {
const npy_timedelta in1 = *(npy_timedelta *)ip1;
const npy_timedelta in2 = *(npy_timedelta *)ip2;
if (in1 == NPY_DATETIME_NAT || in2 == NPY_DATETIME_NAT) {
*((npy_timedelta *)op1) = NPY_DATETIME_NAT;
}
else {
if (in2 == 0) {
npy_set_floatstatus_divbyzero();
*((npy_timedelta *)op1) = 0;
}
else {
/* handle mixed case the way Python does */
const npy_timedelta rem = in1 % in2;
if ((in1 > 0) == (in2 > 0) || rem == 0) {
*((npy_timedelta *)op1) = rem;
}
else {
*((npy_timedelta *)op1) = rem + in2;
}
}
}
}
}

/*
*****************************************************************************
** FLOAT LOOPS **
Expand Down
3 changes: 3 additions & 0 deletions numpy/core/src/umath/loops.h.src
Original file line number Diff line number Diff line change
Expand Up @@ -473,6 +473,9 @@ TIMEDELTA_md_m_divide(char **args, npy_intp *dimensions, npy_intp *steps, void *
NPY_NO_EXPORT void
TIMEDELTA_mm_d_divide(char **args, npy_intp *dimensions, npy_intp *steps, void *NPY_UNUSED(func));

NPY_NO_EXPORT void
TIMEDELTA_mm_m_remainder(char **args, npy_intp *dimensions, npy_intp *steps, void *NPY_UNUSED(func));

/* Special case equivalents to above functions */

#define TIMEDELTA_mq_m_true_divide TIMEDELTA_mq_m_divide
Expand Down
51 changes: 51 additions & 0 deletions numpy/core/src/umath/ufunc_type_resolution.c
Original file line number Diff line number Diff line change
Expand Up @@ -1173,6 +1173,57 @@ PyUFunc_DivisionTypeResolver(PyUFuncObject *ufunc,
}


NPY_NO_EXPORT int
PyUFunc_RemainderTypeResolver(PyUFuncObject *ufunc,
NPY_CASTING casting,
PyArrayObject **operands,
PyObject *type_tup,
PyArray_Descr **out_dtypes)
{
int type_num1, type_num2;
int i;

type_num1 = PyArray_DESCR(operands[0])->type_num;
type_num2 = PyArray_DESCR(operands[1])->type_num;

/* Use the default when datetime and timedelta are not involved */
if (!PyTypeNum_ISDATETIME(type_num1) && !PyTypeNum_ISDATETIME(type_num2)) {
return PyUFunc_DefaultTypeResolver(ufunc, casting, operands,
type_tup, out_dtypes);
}
if (type_num1 == NPY_TIMEDELTA) {
if (type_num2 == NPY_TIMEDELTA) {
Copy link
Member

@eric-wieser eric-wieser Oct 11, 2018

Choose a reason for hiding this comment

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

Why not just write this:

if  (type_num1 == NPY_TIMEDELTA && type_num2 == NPY_TIMEDELTA) {
    // your code
}
else {
    return PyUFunc_DefaultTypeResolver(...)
}

That saves you from having to produce an error message for datetime, making all my above comments moot

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is all just copy-paste from other type resolvers. I was planning to leave room for implementing mq and md remainders, where there would be other switches to handle type_num2 on a case by case basis, so check type_num1 but multiple checks on type_num2.

out_dtypes[0] = PyArray_PromoteTypes(PyArray_DESCR(operands[0]),
PyArray_DESCR(operands[1]));
if (out_dtypes[0] == NULL) {
return -1;
}
out_dtypes[1] = out_dtypes[0];
Py_INCREF(out_dtypes[1]);
out_dtypes[2] = out_dtypes[0];
Py_INCREF(out_dtypes[2]);
}
else {
return raise_binary_type_reso_error(ufunc, operands);
}
}
else {
return raise_binary_type_reso_error(ufunc, operands);
}

/* Check against the casting rules */
if (PyUFunc_ValidateCasting(ufunc, casting, operands, out_dtypes) < 0) {
for (i = 0; i < 3; ++i) {
Py_DECREF(out_dtypes[i]);
out_dtypes[i] = NULL;
}
return -1;
}

return 0;
}


/*
* True division should return float64 results when both inputs are integer
* types. The PyUFunc_DefaultTypeResolver promotes 8 bit integers to float16
Expand Down
7 changes: 7 additions & 0 deletions numpy/core/src/umath/ufunc_type_resolution.h
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,13 @@ PyUFunc_DivisionTypeResolver(PyUFuncObject *ufunc,
PyObject *type_tup,
PyArray_Descr **out_dtypes);

NPY_NO_EXPORT int
PyUFunc_RemainderTypeResolver(PyUFuncObject *ufunc,
NPY_CASTING casting,
PyArrayObject **operands,
PyObject *type_tup,
PyArray_Descr **out_dtypes);

/*
* Does a linear search for the best inner loop of the ufunc.
*
Expand Down
71 changes: 71 additions & 0 deletions numpy/core/tests/test_datetime.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import pytest
from numpy.testing import (
assert_, assert_equal, assert_raises, assert_warns, suppress_warnings,
assert_raises_regex,
)
from numpy.core.numeric import pickle

Expand Down Expand Up @@ -1611,6 +1612,76 @@ def test_timedelta_arange(self):
assert_raises(TypeError, np.arange, np.timedelta64(0, 'Y'),
np.timedelta64(5, 'D'))

@pytest.mark.parametrize("val1, val2, expected", [
# case from gh-12092
(np.timedelta64(7, 's'),
np.timedelta64(3, 's'),
np.timedelta64(1, 's')),
# negative value cases
(np.timedelta64(3, 's'),
np.timedelta64(-2, 's'),
np.timedelta64(-1, 's')),
(np.timedelta64(-3, 's'),
np.timedelta64(2, 's'),
np.timedelta64(1, 's')),
# larger value cases
(np.timedelta64(17, 's'),
np.timedelta64(22, 's'),
np.timedelta64(17, 's')),
(np.timedelta64(22, 's'),
np.timedelta64(17, 's'),
np.timedelta64(5, 's')),
# different units
(np.timedelta64(1, 'm'),
np.timedelta64(57, 's'),
np.timedelta64(3, 's')),
(np.timedelta64(1, 'us'),
np.timedelta64(727, 'ns'),
np.timedelta64(273, 'ns')),
# NaT is propagated
(np.timedelta64('NaT'),
np.timedelta64(50, 'ns'),
np.timedelta64('NaT')),
# Y % M works
(np.timedelta64(2, 'Y'),
np.timedelta64(22, 'M'),
np.timedelta64(2, 'M')),
])
def test_timedelta_modulus(self, val1, val2, expected):
assert_equal(val1 % val2, expected)

@pytest.mark.parametrize("val1, val2", [
# years and months sometimes can't be unambiguously
# divided for modulus operation
(np.timedelta64(7, 'Y'),
np.timedelta64(3, 's')),
(np.timedelta64(7, 'M'),
np.timedelta64(1, 'D')),
])
def test_timedelta_modulus_error(self, val1, val2):
with assert_raises_regex(TypeError, "common metadata divisor"):
val1 % val2

def test_timedelta_modulus_div_by_zero(self):
with assert_warns(RuntimeWarning):
actual = np.timedelta64(10, 's') % np.timedelta64(0, 's')
assert_equal(actual, np.timedelta64(0, 's'))

@pytest.mark.parametrize("val1, val2", [
# cases where one operand is not
# timedelta64
(np.timedelta64(7, 'Y'),
15,),
(7.5,
np.timedelta64(1, 'D')),
])
def test_timedelta_modulus_type_resolution(self, val1, val2):
# NOTE: some of the operations may be supported
# in the future
with assert_raises_regex(TypeError,
"remainder cannot use operands with types"):
val1 % val2

def test_timedelta_arange_no_dtype(self):
d = np.array(5, dtype="m8[D]")
assert_equal(np.arange(d, d + 1), d)
Expand Down