Details
-
Type:
RFC
-
Status: Implemented
-
Resolution: Done
-
Component/s: DM
-
Labels:None
-
Location:this issue page
Description
DM-1197, a request to add type-promoting operators for the Point and Extent classes, generated a long HipChat discussion yesterday on an edge case, and Robert Lupton requested an RFC to go over the proposed changes in detail.
Since then, I've discovered several more edge cases that may be a little controversial, largely in addressing how to reconcile C++'s and Python's differing views of integer division. For the most part, these are problems for which there is no good solution, but I'm also pretty confident there aren't any much better than what I've proposed here.
Here's a table of the complete set of operations I propose supporting, as well as some operations explicitly not supported (indicated with "TypeError"). More extensive notes are below the table.
LHS | RHS | Operator | Result | Notes | |
---|---|---|---|---|---|
PointD | PointD | +, += | TypeError | (1) | |
PointD | PointI | +, += | TypeError | (1) | |
PointD | PointD | - | ExtentD | ||
PointD | PointD | -= | TypeError | (2) | |
PointD | PointI | - | ExtentD | ||
PointD | PointI | -= | TypeError | (2) | |
PointD | ExtentD | +, +=, -, -= | PointD | ||
PointD | ExtentI | +, +=, -, -= | PointD | ||
PointI | PointD | +, += | TypeError | (1) | |
PointI | PointI | +, += | TypeError | (1) | |
PointI | ExtentD | + | PointD | ||
PointI | ExtentD | += | TypeError | (2) | |
PointI | ExtentI | +, += | PointI | ||
PointI | PointD | - | ExtentD | ||
PointI | PointD | -= | TypeError | (2) | |
PointI | PointI | - | ExtentI | ||
PointI | PointI | -= | TypeError | (2) | |
PointI | ExtentD | - | PointD | ||
PointI | ExtentD | -= | TypeError | (2) | |
PointI | ExtentI | -, -= | PointI | ||
ExtentD | PointD | + | PointD | ||
ExtentD | PointD | += | TypeError | (2) | |
ExtentD | PointD | -, -= | TypeError | (1) | |
ExtentD | PointI | + | PointD | ||
ExtentD | PointI | += | TypeError | (2) | |
ExtentD | PointI | -, -= | TypeError | (1) | |
ExtentD | ExtentD | +, +=, -, -= | ExtentD | ||
ExtentD | ExtentI | +, +=, -, -= | ExtentD | ||
ExtentI | PointD | + | PointD | ||
ExtentI | PointD | += | TypeError | (2) | |
ExtentI | PointD | -, -= | TypeError | (1) | |
ExtentI | PointI | + | PointI | ||
ExtentI | PointI | += | TypeError | (2) | |
ExtentI | PointI | -, -= | TypeError | (1) | |
ExtentI | ExtentD | +, - | ExtentD | ||
ExtentI | ExtentD | +=, -= | TypeError | (2) | |
ExtentI | ExtentI | +, -, +=, -= | ExtentI | ||
ExtentD | double | *, *=, /, /= | ExtentD | ||
ExtentD | double | //, //= | TypeError | (7) | |
ExtentD | int | *, *=, /, /= | ExtentD | (3) | |
ExtentD | int | //, //= | TypeError | (7) | |
ExtentI | double | * | ExtentD |
(4) | |
ExtentI | double | *= | TypeError (Python), static_assert failure (C+) |
(2), (4) | |
ExtentI | double | / | ExtentD |
(4), (5), (6) | |
ExtentI | double | /= | TypeError (Python), static_assert failure (C+) |
(2), (4), (5), (6) | |
ExtentI | double | //, //= | TypeError | (7) | |
ExtentI | int | *, *= | ExtentI | ||
ExtentI | int | / | ExtentD | (5), (6) | |
ExtentI | int | /= | TypeError (Python), ExtentI (C++) | (2), (5), (6) | |
ExtentI | int | //, //= | ExtentI | (8) | |
double | ExtentD | * | ExtentD | ||
double | ExtentI | * | ExtentD |
(4) | |
int | ExtentD | * | ExtentD | (3) | |
int | ExtentI | * | ExtentI |
Notes
1. Operation is not geometrically meaningful. Back when we decided to have both Point and Extent classes, the entire purpose was to disallow geometrically meaningless operations like this. If you'd like to revisit that decision, please open a new RFC; that's out of scope for this one.
2. This operator is impossible to implement directly in C++, as it's an in-place operator that would require the LHS type to change. That's actually possible to implement in Python, because Python's in-place operators don't actually have to operate in-place. In fact, if you add a float to an int in-place, you'll change the LHS operand type:
>>> a = 4 |
>>> a += 3.2 |
>>> type(a) |
<type 'float'> |
However, that doesn't work with tuples and lists:
>>> b = (3, 4) |
>>> b += [1, 2] |
Traceback (most recent call last):
|
File "<stdin>", line 1, in <module> |
TypeError: can only concatenate tuple (not "list") to tuple |
Moreover, in NumPy, the operation is allowed but truncates:
>>> c = numpy.array([2], dtype=int) |
>>> c += numpy.array([3.2], dtype=float) |
>>> c
|
array([5]) |
Given the mixed signals from Python, and the potential for unexpected errors (this is not a well-known feature in Python), the proposal here is to not allow in-place operations that can change the type of the LHS, and instead raise TypeError.
3. This operator is not implemented directly in either C++ or Python, but is largely supported by the fact that an overload that takes double will also accept int (but may yield different answers for extremely large integers that cannot be represented exactly as {{double}}s).
4. (Removed - my previous statement about C++ ambiguous overloads was incorrect; see discussion below).
5. All "/" and "/=" operations here assume from _future_ import division. If this is not enabled, the behavior of the "/" operator will be that of "//", and likewise for "/=" and "//=", for all operations with ExtentI on the LHS.
6. That this now returns ExtentD represents an API change in Python, because division of ExtentI by int previously just delegated to the truncating C++ implementation that returns an ExtentI.
7. The // operator applies only to integer types.
8. This Python-only operation does not always produce the same result as regular division of integers in C++, because Python specifies that a // b is equivalent to floor(a / b), while C++ specifies that it should be equivalent to int(a / b). Note that floor rounds negative numbers down and int rounds them up.
Attachments
Issue Links
- blocks
-
DM-1197 Support some mixed-type operations for Point and Extent
- Done
Yes, that's all correct. And avoiding truncation in those in-place operators sounds like an excellent job for static_assert (they'd throw in Python, of course, but in C++ a compile failure would be much more natural).