Uploaded image for project: 'Request For Comments'
  1. Request For Comments
  2. RFC-41

Mixed-Type Operations for Point and Extent

    XMLWordPrintable

    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 (Python), Truncated ExtentI (C++) (4)
      ExtentI double *= TypeError (Python), static_assert failure (C+) Truncated ExtentI (C+) (2), (4)
      ExtentI double / ExtentD (Python), Truncated ExtentI (C++) (4), (5), (6)
      ExtentI double /= TypeError (Python), static_assert failure (C+) Truncated ExtentI (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 (Python), Truncated ExtentI (C++) (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

            Activity

            Hide
            jbosch Jim Bosch added a comment -

            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).

            Show
            jbosch Jim Bosch added a comment - 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).
            Hide
            jbosch Jim Bosch added a comment -

            Now that the ambiguous overloads issue has been resolved, there seems to be a pretty clear consensus here around the proposal, or at least no opinions against it.

            Show
            jbosch Jim Bosch added a comment - Now that the ambiguous overloads issue has been resolved, there seems to be a pretty clear consensus here around the proposal, or at least no opinions against it.
            Hide
            swinbank John Swinbank added a comment -

            While reviewing DM-1197, I attempted to update the table to reflect the results of the discussion above. Jim Bosch, it would probably be a good idea for you to sanity check it. The good news is that most of the differences between the C++ and Python vanish: the only discrepancy is now ExtentI /= int which throws in Python (with future division) but gives you an ExtentI in C++.

            Show
            swinbank John Swinbank added a comment - While reviewing DM-1197 , I attempted to update the table to reflect the results of the discussion above. Jim Bosch , it would probably be a good idea for you to sanity check it. The good news is that most of the differences between the C++ and Python vanish: the only discrepancy is now ExtentI /= int which throws in Python (with future division) but gives you an ExtentI in C++.
            Hide
            jbosch Jim Bosch added a comment -

            Thank for updating the table - sorry I forgot to do that myself. I think the current behavior for ExtentI /= int is probably the best of several imperfect solutions, even if it is a discrepancy between the languages; I think it makes it language behave as users of that language would expect.

            Show
            jbosch Jim Bosch added a comment - Thank for updating the table - sorry I forgot to do that myself. I think the current behavior for ExtentI /= int is probably the best of several imperfect solutions, even if it is a discrepancy between the languages; I think it makes it language behave as users of that language would expect.
            Hide
            jbosch Jim Bosch added a comment -

            Implemented on DM-1197.

            Show
            jbosch Jim Bosch added a comment - Implemented on DM-1197 .

              People

              Assignee:
              jbosch Jim Bosch
              Reporter:
              jbosch Jim Bosch
              Watchers:
              Jim Bosch, John Swinbank, Kian-Tat Lim, Robert Lupton, Russell Owen, Tim Jenness
              Votes:
              0 Vote for this issue
              Watchers:
              6 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved:
                Planned End:

                  Jenkins

                  No builds found.