Adding Arthimetic operations #4
Conversation
| raise TypeError("unsupported operand type(s) for +: '%s' and '%s'" % | ||
| (type(self).__name__, type(y).__name__)) | ||
| else: | ||
| raise TypeError("unsupported operand type(s) for +: '%s' and '%s'" % |
There was a problem hiding this comment.
you can simply return NotImplemented to get Python to raise this default error
There was a problem hiding this comment.
|
I do think it probably makes sense to define the reverse operators, at least for |
|
Just realized that multiplication and division is broken - will fix in a bit ... |
|
Note that this seems to only work for PY3. Not sure what's going on with PY2 ... |
|
That should cover it. @shoyer, do you think the tests for PY2 and PY3 can be specified in the travis test? Otherwise, I think this should be good to go. |
| self.assertEqual(expected, actual) | ||
|
|
||
| expected = Interval(-1, 0) | ||
| actual = 1 - self.interval |
There was a problem hiding this comment.
I don't think this is right -- [0, 1) - 1 should be [-1, 0), but 1 - [0, 1) should be [1, 0), which is not a valid interval.
There was a problem hiding this comment.
Ok. I'll get rid of that test case, as suggested in the comment below.
|
Travis already tests both python 2 and python 3 On Sat, Jan 9, 2016 at 11:47 AM, mortonjt [email protected] wrote:
|
STY: Adding @shoyer's comments BUG: Fixing multiplication and division ENH: Adding in PY2 compatible division TST: Adding PY2 specific test. May want to configure in travis ... TST: Modifying division test to handle both PY2 and PY3
|
Modified the division test case to handle both PY2 and PY3. Let me know if I'm missing anything |
|
looks great -- thanks! |
Let me know if I'm missing anything.
Note, I didn't think it would make sense to have reverse operations (i.e.
1+Interval(1, 2))So that functionality isn't added atm.