Issue 20731: shortcut coercion for Integer-Rational operations

archive/issues_020494.json:

{
    "assignees": [],
    "body": "<div id=\"comment:0\"></div>\n\nDoing some shortcut to coercion actually fasten the code by a factor 3...\n\nOld version\n\n```\nsage: z = 2\nsage: q = QQ((-1,5))\nsage: %timeit z + q\n1000000 loops, best of 3: 930 ns per loop\nsage: %timeit q + z\n1000000 loops, best of 3: 930 ns per loop\nsage: %timeit z - q\n1000000 loops, best of 3: 848 ns per loop\nsage: %timeit q - z\n1000000 loops, best of 3: 776 ns per loop\nsage: %timeit z * q\n1000000 loops, best of 3: 1.15 \u00b5s per loop\nsage: %timeit q * z\n1000000 loops, best of 3: 1.17 \u00b5s per loop\nsage: %timeit z / q\n1000000 loops, best of 3: 1.09 \u00b5s per loop\nsage: %timeit q / z\n1000000 loops, best of 3: 1.1 \u00b5s per loop\n```\nNew version\n\n```\nsage: z = 2\nsage: q = QQ((-1,5))\nsage: %timeit z + q\n1000000 loops, best of 3: 265 ns per loop\nsage: %timeit q + z\n1000000 loops, best of 3: 323 ns per loop\nsage: %timeit z - q\n1000000 loops, best of 3: 261 ns per loop\nsage: %timeit q - z\n1000000 loops, best of 3: 319 ns per loop\nsage: %timeit z * q\n1000000 loops, best of 3: 255 ns per loop\nsage: %timeit q * z\n1000000 loops, best of 3: 332 ns per loop\nsage: %timeit z / q\n1000000 loops, best of 3: 289 ns per loop\nsage: %timeit q / z\n1000000 loops, best of 3: 314 ns per loop\n```\n\nCC:  @jdemeyer\n\nComponent: **coercion**\n\nKeywords: **days74**\n\nAuthor: **Vincent Delecroix**\n\nBranch/Commit: **[`d535aee`](https://github.com/sagemath/sagetrac-mirror/commit/d535aee38e629195fa6319a8a1d1d2efaecebec6)**\n\nReviewer: **Jeroen Demeyer, Travis Scrimshaw**\n\n_Issue created by migration from https://trac.sagemath.org/ticket/20731_\n\n",
    "closed_at": "2016-07-23T18:38:25Z",
    "created_at": "2016-05-31T13:38:42Z",
    "labels": [
        "https://github.com/sagemath/sage/labels/c%3A%20coercion",
        "https://github.com/sagemath/sage/labels/p%3A%20major%20/%203",
        "https://github.com/sagemath/sage/labels/enhancement"
    ],
    "milestone": "https://github.com/sagemath/sage/milestones/sage-7.3",
    "reactions": [],
    "repository": "https://github.com/sagemath/sage",
    "title": "shortcut coercion for Integer-Rational operations",
    "type": "issue",
    "updated_at": "2016-07-23T18:38:25Z",
    "url": "https://github.com/sagemath/sage/issues/20731",
    "user": "https://github.com/videlec"
}

Doing some shortcut to coercion actually fasten the code by a factor 3...

Old version

sage: z = 2
sage: q = QQ((-1,5))
sage: %timeit z + q
1000000 loops, best of 3: 930 ns per loop
sage: %timeit q + z
1000000 loops, best of 3: 930 ns per loop
sage: %timeit z - q
1000000 loops, best of 3: 848 ns per loop
sage: %timeit q - z
1000000 loops, best of 3: 776 ns per loop
sage: %timeit z * q
1000000 loops, best of 3: 1.15 µs per loop
sage: %timeit q * z
1000000 loops, best of 3: 1.17 µs per loop
sage: %timeit z / q
1000000 loops, best of 3: 1.09 µs per loop
sage: %timeit q / z
1000000 loops, best of 3: 1.1 µs per loop

New version

sage: z = 2
sage: q = QQ((-1,5))
sage: %timeit z + q
1000000 loops, best of 3: 265 ns per loop
sage: %timeit q + z
1000000 loops, best of 3: 323 ns per loop
sage: %timeit z - q
1000000 loops, best of 3: 261 ns per loop
sage: %timeit q - z
1000000 loops, best of 3: 319 ns per loop
sage: %timeit z * q
1000000 loops, best of 3: 255 ns per loop
sage: %timeit q * z
1000000 loops, best of 3: 332 ns per loop
sage: %timeit z / q
1000000 loops, best of 3: 289 ns per loop
sage: %timeit q / z
1000000 loops, best of 3: 314 ns per loop

CC: @jdemeyer

Component: coercion

Keywords: days74

Author: Vincent Delecroix

Branch/Commit: d535aee

Reviewer: Jeroen Demeyer, Travis Scrimshaw

Issue created by migration from https://trac.sagemath.org/ticket/20731

archive/issue_events_289307.json:

{
    "actor": "https://github.com/videlec",
    "created_at": "2016-05-31T13:38:42Z",
    "event": "milestoned",
    "issue": "https://github.com/sagemath/sage/issues/20731",
    "milestone_number": null,
    "milestone_title": "sage-7.3",
    "type": "issue_event",
    "url": "https://github.com/sagemath/sage/issues/20731#event-289307"
}

archive/issue_events_289308.json:

{
    "actor": "https://github.com/videlec",
    "created_at": "2016-05-31T13:38:42Z",
    "event": "labeled",
    "issue": "https://github.com/sagemath/sage/issues/20731",
    "label": "https://github.com/sagemath/sage/labels/c%3A%20coercion",
    "label_color": "0000ff",
    "label_name": "c: coercion",
    "label_text_color": "ffffff",
    "type": "issue_event",
    "url": "https://github.com/sagemath/sage/issues/20731#event-289308"
}

archive/issue_events_289309.json:

{
    "actor": "https://github.com/videlec",
    "created_at": "2016-05-31T13:38:42Z",
    "event": "labeled",
    "issue": "https://github.com/sagemath/sage/issues/20731",
    "label": "https://github.com/sagemath/sage/labels/p%3A%20major%20/%203",
    "label_color": "ffbb00",
    "label_name": "p: major / 3",
    "label_text_color": "ffffff",
    "type": "issue_event",
    "url": "https://github.com/sagemath/sage/issues/20731#event-289309"
}

archive/issue_events_289310.json:

{
    "actor": "https://github.com/videlec",
    "created_at": "2016-05-31T13:38:42Z",
    "event": "labeled",
    "issue": "https://github.com/sagemath/sage/issues/20731",
    "label": "https://github.com/sagemath/sage/labels/enhancement",
    "label_color": "696969",
    "label_name": "enhancement",
    "label_text_color": "ffffff",
    "type": "issue_event",
    "url": "https://github.com/sagemath/sage/issues/20731#event-289310"
}

archive/issue_comments_300191.json:

{
    "body": "Description changed:\n``````diff\n--- \n+++ \n@@ -1,18 +1,26 @@\n-Doing some shortcut to coercion actually fasten the code\n+Doing some shortcut to coercion actually fasten the code by a factor 3...\n \n Old version\n \n ```\n sage: z = 2\n sage: q = QQ((-1,5))\n-sage: %timeit z + q\n-sage: %timeit q + z\n-sage: %timeit z - q\n-sage: %timeit q - z\n-sage: %timeit z * q\n-sage: %timeit q * z\n-sage: %timeit z / q\n-sage: %timeit q / z\n+sage: sage: %timeit z + q\n+1000000 loops, best of 3: 930 ns per loop\n+sage: sage: %timeit q + z\n+1000000 loops, best of 3: 930 ns per loop\n+sage: sage: %timeit z - q\n+1000000 loops, best of 3: 848 ns per loop\n+sage: sage: %timeit q - z\n+1000000 loops, best of 3: 776 ns per loop\n+sage: sage: %timeit z * q\n+1000000 loops, best of 3: 1.15 \u00b5s per loop\n+sage: sage: %timeit q * z\n+1000000 loops, best of 3: 1.17 \u00b5s per loop\n+sage: sage: %timeit z / q\n+1000000 loops, best of 3: 1.09 \u00b5s per loop\n+sage: sage: %timeit q / z\n+1000000 loops, best of 3: 1.1 \u00b5s per loop\n ```\n New version\n \n``````\n",
    "created_at": "2016-05-31T13:44:41Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20731",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20731#issuecomment-300191",
    "user": "https://github.com/videlec"
}

Description changed:

--- 
+++ 
@@ -1,18 +1,26 @@
-Doing some shortcut to coercion actually fasten the code
+Doing some shortcut to coercion actually fasten the code by a factor 3...
 
 Old version
 
 ```
 sage: z = 2
 sage: q = QQ((-1,5))
-sage: %timeit z + q
-sage: %timeit q + z
-sage: %timeit z - q
-sage: %timeit q - z
-sage: %timeit z * q
-sage: %timeit q * z
-sage: %timeit z / q
-sage: %timeit q / z
+sage: sage: %timeit z + q
+1000000 loops, best of 3: 930 ns per loop
+sage: sage: %timeit q + z
+1000000 loops, best of 3: 930 ns per loop
+sage: sage: %timeit z - q
+1000000 loops, best of 3: 848 ns per loop
+sage: sage: %timeit q - z
+1000000 loops, best of 3: 776 ns per loop
+sage: sage: %timeit z * q
+1000000 loops, best of 3: 1.15 µs per loop
+sage: sage: %timeit q * z
+1000000 loops, best of 3: 1.17 µs per loop
+sage: sage: %timeit z / q
+1000000 loops, best of 3: 1.09 µs per loop
+sage: sage: %timeit q / z
+1000000 loops, best of 3: 1.1 µs per loop
 ```
 New version

archive/issue_comments_300192.json:

{
    "body": "Branch: **[u/vdelecroix/20731](https://github.com/sagemath/sagetrac-mirror/tree/u/vdelecroix/20731)**",
    "created_at": "2016-05-31T13:44:41Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20731",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20731#issuecomment-300192",
    "user": "https://github.com/videlec"
}

Branch: u/vdelecroix/20731

archive/issue_events_289311.json:

{
    "actor": "https://github.com/videlec",
    "created_at": "2016-05-31T13:44:41Z",
    "event": "labeled",
    "issue": "https://github.com/sagemath/sage/issues/20731",
    "label": "https://github.com/sagemath/sage/labels/needs%20review",
    "label_color": "7fff00",
    "label_name": "needs review",
    "label_text_color": "ffffff",
    "type": "issue_event",
    "url": "https://github.com/sagemath/sage/issues/20731#event-289311"
}

archive/issue_comments_300193.json:

{
    "body": "<div id=\"comment:2\"></div>\n\nBranch pushed to git repo; I updated commit sha1. New commits:\n<table><tr><td><a href=\"https://github.com/sagemath/sagetrac-mirror/commit/33f9fd432ff127a5234b120bebda1f7b63bb0f84\"><code>33f9fd4</code></a></td><td><code>Trac 20731: shortcut coercion for Integer-Rational</code></td></tr></table>\n",
    "created_at": "2016-05-31T13:44:45Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20731",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20731#issuecomment-300193",
    "user": "https://github.com/sagetrac-git"
}

Branch pushed to git repo; I updated commit sha1. New commits:

33f9fd4 Trac 20731: shortcut coercion for Integer-Rational

archive/issue_comments_300194.json:

{
    "body": "Commit: **[`33f9fd4`](https://github.com/sagemath/sagetrac-mirror/commit/33f9fd432ff127a5234b120bebda1f7b63bb0f84)**",
    "created_at": "2016-05-31T13:44:45Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20731",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20731#issuecomment-300194",
    "user": "https://github.com/sagetrac-git"
}

Commit: 33f9fd4

archive/issue_comments_300195.json:

{
    "body": "Changed commit from **[`33f9fd4`](https://github.com/sagemath/sagetrac-mirror/commit/33f9fd432ff127a5234b120bebda1f7b63bb0f84)** to **[`19c075b`](https://github.com/sagemath/sagetrac-mirror/commit/19c075bf18073049fea1607b483c591feef7df22)**",
    "created_at": "2016-05-31T19:20:28Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20731",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20731#issuecomment-300195",
    "user": "https://github.com/sagetrac-git"
}

Changed commit from 33f9fd4 to 19c075b

archive/issue_comments_300196.json:

{
    "body": "<div id=\"comment:3\"></div>\n\nBranch pushed to git repo; I updated commit sha1. New commits:\n<table><tr><td><a href=\"https://github.com/sagemath/sagetrac-mirror/commit/19c075bf18073049fea1607b483c591feef7df22\"><code>19c075b</code></a></td><td><code>Trac 20731: fix doctests</code></td></tr></table>\n",
    "created_at": "2016-05-31T19:20:28Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20731",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20731#issuecomment-300196",
    "user": "https://github.com/sagetrac-git"
}

Branch pushed to git repo; I updated commit sha1. New commits:

19c075b Trac 20731: fix doctests

archive/issue_comments_300197.json:

{
    "body": "<div id=\"comment:4\" align=\"right\">comment:4</div>\n\nMove `src/sage/rings/binop.pyx` to `src/sage/libs/gmp` since it has nothing to with rings. And I would just use a `.pxd` file with inline functions, no `.pyx` file.",
    "created_at": "2016-06-01T08:21:40Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20731",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20731#issuecomment-300197",
    "user": "https://github.com/jdemeyer"
}

comment:4

Move src/sage/rings/binop.pyx to src/sage/libs/gmp since it has nothing to with rings. And I would just use a .pxd file with inline functions, no .pyx file.

archive/issue_comments_300198.json:

{
    "body": "<div id=\"comment:5\"></div>\n\nBranch pushed to git repo; I updated commit sha1. New commits:\n<table><tr><td><a href=\"https://github.com/sagemath/sagetrac-mirror/commit/fdf79acfa3433ef167ee3e865c16dd429caf24e6\"><code>fdf79ac</code></a></td><td><code>Trac 20731: get rid of ZZ._div</code></td></tr></table>\n",
    "created_at": "2016-06-01T08:26:01Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20731",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20731#issuecomment-300198",
    "user": "https://github.com/sagetrac-git"
}

Branch pushed to git repo; I updated commit sha1. New commits:

fdf79ac Trac 20731: get rid of ZZ._div

archive/issue_comments_300199.json:

{
    "body": "Changed commit from **[`19c075b`](https://github.com/sagemath/sagetrac-mirror/commit/19c075bf18073049fea1607b483c591feef7df22)** to **[`fdf79ac`](https://github.com/sagemath/sagetrac-mirror/commit/fdf79acfa3433ef167ee3e865c16dd429caf24e6)**",
    "created_at": "2016-06-01T08:26:01Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20731",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20731#issuecomment-300199",
    "user": "https://github.com/sagetrac-git"
}

Changed commit from 19c075b to fdf79ac

archive/issue_comments_300200.json:

{
    "body": "Changed commit from **[`fdf79ac`](https://github.com/sagemath/sagetrac-mirror/commit/fdf79acfa3433ef167ee3e865c16dd429caf24e6)** to **[`eec4168`](https://github.com/sagemath/sagetrac-mirror/commit/eec41681c0fc9bb3b377ad67a31634a40d277782)**",
    "created_at": "2016-06-01T08:30:51Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20731",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20731#issuecomment-300200",
    "user": "https://github.com/sagetrac-git"
}

Changed commit from fdf79ac to eec4168

archive/issue_comments_300201.json:

{
    "body": "<div id=\"comment:6\"></div>\n\nBranch pushed to git repo; I updated commit sha1. New commits:\n<table><tr><td><a href=\"https://github.com/sagemath/sagetrac-mirror/commit/9078830d92c33d8a2e06ebb065e113e97a31d0e1\"><code>9078830</code></a></td><td><code>Trac 20731: moving binop</code></td></tr><tr><td><a href=\"https://github.com/sagemath/sagetrac-mirror/commit/eec41681c0fc9bb3b377ad67a31634a40d277782\"><code>eec4168</code></a></td><td><code>Trac 20731: fix the move</code></td></tr></table>\n",
    "created_at": "2016-06-01T08:30:51Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20731",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20731#issuecomment-300201",
    "user": "https://github.com/sagetrac-git"
}

Branch pushed to git repo; I updated commit sha1. New commits:

`9078830`	`Trac 20731: moving binop`
`eec4168`	`Trac 20731: fix the move`

archive/issue_comments_300202.json:

{
    "body": "<div id=\"comment:7\" align=\"right\">comment:7</div>\n\nI think that `mpq_mul_z` and `mpq_div_z` are overkill. Why don't you just multiply the numerator or denominator with the integer and call `mpq_canonicalize`, similar to what you do for `Integer/Integer` division?",
    "created_at": "2016-06-01T15:52:51Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20731",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20731#issuecomment-300202",
    "user": "https://github.com/jdemeyer"
}

comment:7

I think that mpq_mul_z and mpq_div_z are overkill. Why don't you just multiply the numerator or denominator with the integer and call mpq_canonicalize, similar to what you do for Integer/Integer division?

archive/issue_events_289312.json:

{
    "actor": "https://github.com/jdemeyer",
    "created_at": "2016-06-01T15:52:51Z",
    "event": "unlabeled",
    "issue": "https://github.com/sagemath/sage/issues/20731",
    "label": "https://github.com/sagemath/sage/labels/needs%20review",
    "label_color": "7fff00",
    "label_name": "needs review",
    "label_text_color": "ffffff",
    "type": "issue_event",
    "url": "https://github.com/sagemath/sage/issues/20731#event-289312"
}

archive/issue_events_289313.json:

{
    "actor": "https://github.com/jdemeyer",
    "created_at": "2016-06-01T15:52:51Z",
    "event": "labeled",
    "issue": "https://github.com/sagemath/sage/issues/20731",
    "label": "https://github.com/sagemath/sage/labels/needs%20work",
    "label_color": "ffff00",
    "label_name": "needs work",
    "label_text_color": "ffffff",
    "type": "issue_event",
    "url": "https://github.com/sagemath/sage/issues/20731#event-289313"
}

archive/issue_comments_300203.json:

{
    "body": "<div id=\"comment:8\" align=\"right\">comment:8</div>\n\n`QQ((2^100, 3^100)) * 3^100`",
    "created_at": "2016-06-01T16:24:46Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20731",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20731#issuecomment-300203",
    "user": "https://github.com/videlec"
}

comment:8

QQ((2^100, 3^100)) * 3^100

archive/issue_comments_300204.json:

{
    "body": "<div id=\"comment:9\" align=\"right\">comment:9</div>\n\nRight, but that's a rather artificial example. What about simple cases?",
    "created_at": "2016-06-01T16:28:54Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20731",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20731#issuecomment-300204",
    "user": "https://github.com/jdemeyer"
}

comment:9

Right, but that's a rather artificial example. What about simple cases?

archive/issue_comments_300205.json:

{
    "body": "<div id=\"comment:10\" align=\"right\">comment:10</div>\n\nIn `src/sage/repl/interpreter.py`, there is a good reason that the line numbers were changed to `...`",
    "created_at": "2016-06-01T16:30:04Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20731",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20731#issuecomment-300205",
    "user": "https://github.com/jdemeyer"
}

comment:10

In src/sage/repl/interpreter.py, there is a good reason that the line numbers were changed to ...

archive/issue_comments_300206.json:

{
    "body": "<div id=\"comment:11\" align=\"right\">comment:11</div>\n\nI just thought about the following very simple algorithm for the multiplication (division is analogous):\n\nTo compute `(A/B) * C`, you initialize the result as `C/B`, canonicalize it and then multiply the numerator by `A`.\n\nYou don't need to canonicalize the final result, since `gcd(A,B) = 1` by assumption.",
    "created_at": "2016-06-01T19:55:20Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20731",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20731#issuecomment-300206",
    "user": "https://github.com/jdemeyer"
}

comment:11

I just thought about the following very simple algorithm for the multiplication (division is analogous):

To compute (A/B) * C, you initialize the result as C/B, canonicalize it and then multiply the numerator by A.

You don't need to canonicalize the final result, since gcd(A,B) = 1 by assumption.

archive/issue_comments_300207.json:

{
    "body": "<div id=\"comment:12\"></div>\n\nBranch pushed to git repo; I updated commit sha1. New commits:\n<table><tr><td><a href=\"https://github.com/sagemath/sagetrac-mirror/commit/72f77b99ad55fd2ba186d33c69010214fa569405\"><code>72f77b9</code></a></td><td><code>Trac 20731: simpler operations</code></td></tr><tr><td><a href=\"https://github.com/sagemath/sagetrac-mirror/commit/c7a8d8e5b44d49654aab2827f3b5359f4c7acd60\"><code>c7a8d8e</code></a></td><td><code>20731: simpler mpq_mul_z/mpq_div_z</code></td></tr></table>\n",
    "created_at": "2016-06-02T02:44:24Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20731",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20731#issuecomment-300207",
    "user": "https://github.com/sagetrac-git"
}

Branch pushed to git repo; I updated commit sha1. New commits:

`72f77b9`	`Trac 20731: simpler operations`
`c7a8d8e`	`20731: simpler mpq_mul_z/mpq_div_z`

archive/issue_comments_300208.json:

{
    "body": "Changed commit from **[`eec4168`](https://github.com/sagemath/sagetrac-mirror/commit/eec41681c0fc9bb3b377ad67a31634a40d277782)** to **[`c7a8d8e`](https://github.com/sagemath/sagetrac-mirror/commit/c7a8d8e5b44d49654aab2827f3b5359f4c7acd60)**",
    "created_at": "2016-06-02T02:44:24Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20731",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20731#issuecomment-300208",
    "user": "https://github.com/sagetrac-git"
}

Changed commit from eec4168 to c7a8d8e

archive/issue_events_289314.json:

{
    "actor": "https://github.com/videlec",
    "created_at": "2016-06-02T07:19:23Z",
    "event": "unlabeled",
    "issue": "https://github.com/sagemath/sage/issues/20731",
    "label": "https://github.com/sagemath/sage/labels/needs%20work",
    "label_color": "ffff00",
    "label_name": "needs work",
    "label_text_color": "ffffff",
    "type": "issue_event",
    "url": "https://github.com/sagemath/sage/issues/20731#event-289314"
}

archive/issue_events_289315.json:

{
    "actor": "https://github.com/videlec",
    "created_at": "2016-06-02T07:19:23Z",
    "event": "labeled",
    "issue": "https://github.com/sagemath/sage/issues/20731",
    "label": "https://github.com/sagemath/sage/labels/needs%20review",
    "label_color": "7fff00",
    "label_name": "needs review",
    "label_text_color": "ffffff",
    "type": "issue_event",
    "url": "https://github.com/sagemath/sage/issues/20731#event-289315"
}

archive/issue_comments_300209.json:

{
    "body": "<div id=\"comment:14\" align=\"right\">comment:14</div>\n\n[comment:10]",
    "created_at": "2016-06-02T08:13:58Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20731",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20731#issuecomment-300209",
    "user": "https://github.com/jdemeyer"
}

comment:14

[comment:10]

archive/issue_events_289316.json:

{
    "actor": "https://github.com/jdemeyer",
    "created_at": "2016-06-02T08:13:58Z",
    "event": "unlabeled",
    "issue": "https://github.com/sagemath/sage/issues/20731",
    "label": "https://github.com/sagemath/sage/labels/needs%20review",
    "label_color": "7fff00",
    "label_name": "needs review",
    "label_text_color": "ffffff",
    "type": "issue_event",
    "url": "https://github.com/sagemath/sage/issues/20731#event-289316"
}

archive/issue_events_289317.json:

{
    "actor": "https://github.com/jdemeyer",
    "created_at": "2016-06-02T08:13:58Z",
    "event": "labeled",
    "issue": "https://github.com/sagemath/sage/issues/20731",
    "label": "https://github.com/sagemath/sage/labels/needs%20work",
    "label_color": "ffff00",
    "label_name": "needs work",
    "label_text_color": "ffffff",
    "type": "issue_event",
    "url": "https://github.com/sagemath/sage/issues/20731#event-289317"
}

archive/issue_comments_300210.json:

{
    "body": "<div id=\"comment:15\" align=\"right\">comment:15</div>\n\nWhich line numbers?",
    "created_at": "2016-06-02T08:19:59Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20731",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20731#issuecomment-300210",
    "user": "https://github.com/videlec"
}

comment:15

Which line numbers?

archive/issue_comments_300211.json:

{
    "body": "<div id=\"comment:16\" align=\"right\">comment:16</div>\n\nI see...",
    "created_at": "2016-06-02T08:20:26Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20731",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20731#issuecomment-300211",
    "user": "https://github.com/videlec"
}

comment:16

I see...

archive/issue_comments_300212.json:

{
    "body": "Changed commit from **[`c7a8d8e`](https://github.com/sagemath/sagetrac-mirror/commit/c7a8d8e5b44d49654aab2827f3b5359f4c7acd60)** to **[`97a809e`](https://github.com/sagemath/sagetrac-mirror/commit/97a809ecbc63d4dad9c794af1b1fda089fece267)**",
    "created_at": "2016-06-02T08:25:45Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20731",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20731#issuecomment-300212",
    "user": "https://github.com/sagetrac-git"
}

Changed commit from c7a8d8e to 97a809e

archive/issue_comments_300213.json:

{
    "body": "<div id=\"comment:17\"></div>\n\nBranch pushed to git repo; I updated commit sha1. New commits:\n<table><tr><td><a href=\"https://github.com/sagemath/sagetrac-mirror/commit/97a809ecbc63d4dad9c794af1b1fda089fece267\"><code>97a809e</code></a></td><td><code>Trac 20731: remove line numbers in a doctest</code></td></tr></table>\n",
    "created_at": "2016-06-02T08:25:45Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20731",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20731#issuecomment-300213",
    "user": "https://github.com/sagetrac-git"
}

Branch pushed to git repo; I updated commit sha1. New commits:

97a809e Trac 20731: remove line numbers in a doctest

archive/issue_events_289318.json:

{
    "actor": "https://github.com/videlec",
    "created_at": "2016-06-02T08:26:19Z",
    "event": "unlabeled",
    "issue": "https://github.com/sagemath/sage/issues/20731",
    "label": "https://github.com/sagemath/sage/labels/needs%20work",
    "label_color": "ffff00",
    "label_name": "needs work",
    "label_text_color": "ffffff",
    "type": "issue_event",
    "url": "https://github.com/sagemath/sage/issues/20731#event-289318"
}

archive/issue_events_289319.json:

{
    "actor": "https://github.com/videlec",
    "created_at": "2016-06-02T08:26:19Z",
    "event": "labeled",
    "issue": "https://github.com/sagemath/sage/issues/20731",
    "label": "https://github.com/sagemath/sage/labels/needs%20review",
    "label_color": "7fff00",
    "label_name": "needs review",
    "label_text_color": "ffffff",
    "type": "issue_event",
    "url": "https://github.com/sagemath/sage/issues/20731#event-289319"
}

archive/issue_events_289320.json:

{
    "actor": "https://github.com/jdemeyer",
    "created_at": "2016-06-02T08:36:49Z",
    "event": "unlabeled",
    "issue": "https://github.com/sagemath/sage/issues/20731",
    "label": "https://github.com/sagemath/sage/labels/needs%20review",
    "label_color": "7fff00",
    "label_name": "needs review",
    "label_text_color": "ffffff",
    "type": "issue_event",
    "url": "https://github.com/sagemath/sage/issues/20731#event-289320"
}

archive/issue_events_289321.json:

{
    "actor": "https://github.com/jdemeyer",
    "created_at": "2016-06-02T08:36:49Z",
    "event": "labeled",
    "issue": "https://github.com/sagemath/sage/issues/20731",
    "label": "https://github.com/sagemath/sage/labels/needs%20work",
    "label_color": "ffff00",
    "label_name": "needs work",
    "label_text_color": "ffffff",
    "type": "issue_event",
    "url": "https://github.com/sagemath/sage/issues/20731#event-289321"
}

archive/issue_comments_300214.json:

{
    "body": "<div id=\"comment:19\" align=\"right\">comment:19</div>\n\n`12883`",
    "created_at": "2016-06-02T08:36:49Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20731",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20731#issuecomment-300214",
    "user": "https://github.com/jdemeyer"
}

comment:19

12883

archive/issue_comments_300215.json:

{
    "body": "<div id=\"comment:20\"></div>\n\nBranch pushed to git repo; I updated commit sha1. New commits:\n<table><tr><td><a href=\"https://github.com/sagemath/sagetrac-mirror/commit/647a17bc7965f7c5daa91490d311eea894a31983\"><code>647a17b</code></a></td><td><code>Trac 20731: one more line number</code></td></tr></table>\n",
    "created_at": "2016-06-02T08:40:56Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20731",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20731#issuecomment-300215",
    "user": "https://github.com/sagetrac-git"
}

Branch pushed to git repo; I updated commit sha1. New commits:

647a17b Trac 20731: one more line number

archive/issue_comments_300216.json:

{
    "body": "Changed commit from **[`97a809e`](https://github.com/sagemath/sagetrac-mirror/commit/97a809ecbc63d4dad9c794af1b1fda089fece267)** to **[`647a17b`](https://github.com/sagemath/sagetrac-mirror/commit/647a17bc7965f7c5daa91490d311eea894a31983)**",
    "created_at": "2016-06-02T08:40:56Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20731",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20731#issuecomment-300216",
    "user": "https://github.com/sagetrac-git"
}

Changed commit from 97a809e to 647a17b

archive/issue_events_289322.json:

{
    "actor": "https://github.com/videlec",
    "created_at": "2016-06-02T08:41:03Z",
    "event": "unlabeled",
    "issue": "https://github.com/sagemath/sage/issues/20731",
    "label": "https://github.com/sagemath/sage/labels/needs%20work",
    "label_color": "ffff00",
    "label_name": "needs work",
    "label_text_color": "ffffff",
    "type": "issue_event",
    "url": "https://github.com/sagemath/sage/issues/20731#event-289322"
}

archive/issue_events_289323.json:

{
    "actor": "https://github.com/videlec",
    "created_at": "2016-06-02T08:41:03Z",
    "event": "labeled",
    "issue": "https://github.com/sagemath/sage/issues/20731",
    "label": "https://github.com/sagemath/sage/labels/needs%20review",
    "label_color": "7fff00",
    "label_name": "needs review",
    "label_text_color": "ffffff",
    "type": "issue_event",
    "url": "https://github.com/sagemath/sage/issues/20731#event-289323"
}

archive/issue_comments_300217.json:

{
    "body": "<div id=\"comment:22\" align=\"right\">comment:22</div>\n\nGiven that the block\n\n```\n        mpz_set(mpq_numref(X), A)\n        mpz_set(mpq_denref(X), B)\n        mpq_canonicalize(X)\n```\noccurs many times, you should probably make it a separate inline function too, let's call it `mpq_div_zz`.",
    "created_at": "2016-06-02T09:34:30Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20731",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20731#issuecomment-300217",
    "user": "https://github.com/jdemeyer"
}

comment:22

Given that the block

        mpz_set(mpq_numref(X), A)
        mpz_set(mpq_denref(X), B)
        mpq_canonicalize(X)

occurs many times, you should probably make it a separate inline function too, let's call it mpq_div_zz.

archive/issue_comments_300218.json:

{
    "body": "<div id=\"comment:23\" align=\"right\">comment:23</div>\n\nI'm still not convinced by the `if type(x) is type(y)`. That would break if somebody wants to subclass `Integer` for some reason. You can check the types first as an optimization, but you should still support the case where `isinstance(x, Integer) and isinstance(y, Integer)`.",
    "created_at": "2016-06-02T09:42:15Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20731",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20731#issuecomment-300218",
    "user": "https://github.com/jdemeyer"
}

comment:23

I'm still not convinced by the if type(x) is type(y). That would break if somebody wants to subclass Integer for some reason. You can check the types first as an optimization, but you should still support the case where isinstance(x, Integer) and isinstance(y, Integer).

archive/issue_comments_300219.json:

{
    "body": "<div id=\"comment:24\"></div>\n\nBranch pushed to git repo; I updated commit sha1. New commits:\n<table><tr><td><a href=\"https://github.com/sagemath/sagetrac-mirror/commit/675f8e617bdcc66e718d18089938b8d1458d0724\"><code>675f8e6</code></a></td><td><code>Trac 20731: some factorization</code></td></tr></table>\n",
    "created_at": "2016-06-02T12:34:07Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20731",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20731#issuecomment-300219",
    "user": "https://github.com/sagetrac-git"
}

Branch pushed to git repo; I updated commit sha1. New commits:

675f8e6 Trac 20731: some factorization

archive/issue_comments_300220.json:

{
    "body": "Changed commit from **[`647a17b`](https://github.com/sagemath/sagetrac-mirror/commit/647a17bc7965f7c5daa91490d311eea894a31983)** to **[`675f8e6`](https://github.com/sagemath/sagetrac-mirror/commit/675f8e617bdcc66e718d18089938b8d1458d0724)**",
    "created_at": "2016-06-02T12:34:07Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20731",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20731#issuecomment-300220",
    "user": "https://github.com/sagetrac-git"
}

Changed commit from 647a17b to 675f8e6

archive/issue_comments_300221.json:

{
    "body": "<div id=\"comment:25\" align=\"right\">comment:25</div>\n\nReplying to [@jdemeyer](#comment%3A23):\n> I'm still not convinced by the `if type(x) is type(y)`. That would break if somebody wants to subclass `Integer` for some reason. You can check the types first as an optimization, but you should still support the case where `isinstance(x, Integer) and isinstance(y, Integer)`.\n\nIt perfectly works ;-)\n\n```\nsage: class A(Integer): pass\nsage: A() + 1\n1\nsage: 1 + A()\n1\n```",
    "created_at": "2016-06-02T13:11:39Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20731",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20731#issuecomment-300221",
    "user": "https://github.com/videlec"
}

comment:25

Replying to @jdemeyer:

I'm still not convinced by the if type(x) is type(y). That would break if somebody wants to subclass Integer for some reason. You can check the types first as an optimization, but you should still support the case where isinstance(x, Integer) and isinstance(y, Integer).

It perfectly works ;-)

sage: class A(Integer): pass
sage: A() + 1
1
sage: 1 + A()
1

archive/issue_comments_300222.json:

{
    "body": "Description changed:\n``````diff\n--- \n+++ \n@@ -5,21 +5,21 @@\n ```\n sage: z = 2\n sage: q = QQ((-1,5))\n-sage: sage: %timeit z + q\n+sage: %timeit z + q\n 1000000 loops, best of 3: 930 ns per loop\n-sage: sage: %timeit q + z\n+sage: %timeit q + z\n 1000000 loops, best of 3: 930 ns per loop\n-sage: sage: %timeit z - q\n+sage: %timeit z - q\n 1000000 loops, best of 3: 848 ns per loop\n-sage: sage: %timeit q - z\n+sage: %timeit q - z\n 1000000 loops, best of 3: 776 ns per loop\n-sage: sage: %timeit z * q\n+sage: %timeit z * q\n 1000000 loops, best of 3: 1.15 \u00b5s per loop\n-sage: sage: %timeit q * z\n+sage: %timeit q * z\n 1000000 loops, best of 3: 1.17 \u00b5s per loop\n-sage: sage: %timeit z / q\n+sage: %timeit z / q\n 1000000 loops, best of 3: 1.09 \u00b5s per loop\n-sage: sage: %timeit q / z\n+sage: %timeit q / z\n 1000000 loops, best of 3: 1.1 \u00b5s per loop\n ```\n New version\n@@ -36,11 +36,11 @@\n sage: %timeit q - z\n 1000000 loops, best of 3: 319 ns per loop\n sage: %timeit z * q\n-1000000 loops, best of 3: 341 ns per loop\n+1000000 loops, best of 3: 255 ns per loop\n sage: %timeit q * z\n-1000000 loops, best of 3: 446 ns per loop\n+1000000 loops, best of 3: 332 ns per loop\n sage: %timeit z / q\n-1000000 loops, best of 3: 341 ns per loop\n+1000000 loops, best of 3: 289 ns per loop\n sage: %timeit q / z\n-1000000 loops, best of 3: 396 ns per loop\n+1000000 loops, best of 3: 314 ns per loop\n ```\n``````\n",
    "created_at": "2016-06-02T13:13:10Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20731",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20731#issuecomment-300222",
    "user": "https://github.com/videlec"
}

Description changed:

--- 
+++ 
@@ -5,21 +5,21 @@
 ```
 sage: z = 2
 sage: q = QQ((-1,5))
-sage: sage: %timeit z + q
+sage: %timeit z + q
 1000000 loops, best of 3: 930 ns per loop
-sage: sage: %timeit q + z
+sage: %timeit q + z
 1000000 loops, best of 3: 930 ns per loop
-sage: sage: %timeit z - q
+sage: %timeit z - q
 1000000 loops, best of 3: 848 ns per loop
-sage: sage: %timeit q - z
+sage: %timeit q - z
 1000000 loops, best of 3: 776 ns per loop
-sage: sage: %timeit z * q
+sage: %timeit z * q
 1000000 loops, best of 3: 1.15 µs per loop
-sage: sage: %timeit q * z
+sage: %timeit q * z
 1000000 loops, best of 3: 1.17 µs per loop
-sage: sage: %timeit z / q
+sage: %timeit z / q
 1000000 loops, best of 3: 1.09 µs per loop
-sage: sage: %timeit q / z
+sage: %timeit q / z
 1000000 loops, best of 3: 1.1 µs per loop
 ```
 New version
@@ -36,11 +36,11 @@
 sage: %timeit q - z
 1000000 loops, best of 3: 319 ns per loop
 sage: %timeit z * q
-1000000 loops, best of 3: 341 ns per loop
+1000000 loops, best of 3: 255 ns per loop
 sage: %timeit q * z
-1000000 loops, best of 3: 446 ns per loop
+1000000 loops, best of 3: 332 ns per loop
 sage: %timeit z / q
-1000000 loops, best of 3: 341 ns per loop
+1000000 loops, best of 3: 289 ns per loop
 sage: %timeit q / z
-1000000 loops, best of 3: 396 ns per loop
+1000000 loops, best of 3: 314 ns per loop
 ```

archive/issue_comments_300223.json:

{
    "body": "<div id=\"comment:27\" align=\"right\">comment:27</div>\n\nRight, subclassing `Integer` works because `__add__` calls the coercion model and the coercion model calls `_add_` (single underscore).",
    "created_at": "2016-06-03T08:25:13Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20731",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20731#issuecomment-300223",
    "user": "https://github.com/jdemeyer"
}

comment:27

Right, subclassing Integer works because __add__ calls the coercion model and the coercion model calls _add_ (single underscore).

archive/issue_comments_300224.json:

{
    "body": "<div id=\"comment:28\" align=\"right\">comment:28</div>\n\nSome ---hopefully final--- notes:\n\n1. Check documentation (for example, `Rational.__sub__` documents plus)\n\n2. In `mpq_div_z`, you do not need to check the sign since the denominator is the product of two denominators, so it must be positive.\n\n3. I would remove functions `mpq_sub_z` and `mpq_div_z` since they do not commute and are less useful: just implement them directly. Then you can also avoid `mpq_neg` and `mpq_inv` if you do it correctly.",
    "created_at": "2016-06-03T08:33:20Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20731",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20731#issuecomment-300224",
    "user": "https://github.com/jdemeyer"
}

comment:28

Some ---hopefully final--- notes:

Check documentation (for example, Rational.__sub__ documents plus)
In mpq_div_z, you do not need to check the sign since the denominator is the product of two denominators, so it must be positive.
I would remove functions mpq_sub_z and mpq_div_z since they do not commute and are less useful: just implement them directly. Then you can also avoid mpq_neg and mpq_inv if you do it correctly.

archive/issue_events_289324.json:

{
    "actor": "https://github.com/jdemeyer",
    "created_at": "2016-06-03T08:33:20Z",
    "event": "unlabeled",
    "issue": "https://github.com/sagemath/sage/issues/20731",
    "label": "https://github.com/sagemath/sage/labels/needs%20review",
    "label_color": "7fff00",
    "label_name": "needs review",
    "label_text_color": "ffffff",
    "type": "issue_event",
    "url": "https://github.com/sagemath/sage/issues/20731#event-289324"
}

archive/issue_events_289325.json:

{
    "actor": "https://github.com/jdemeyer",
    "created_at": "2016-06-03T08:33:20Z",
    "event": "labeled",
    "issue": "https://github.com/sagemath/sage/issues/20731",
    "label": "https://github.com/sagemath/sage/labels/needs%20work",
    "label_color": "ffff00",
    "label_name": "needs work",
    "label_text_color": "ffffff",
    "type": "issue_event",
    "url": "https://github.com/sagemath/sage/issues/20731#event-289325"
}

archive/issue_comments_300225.json:

{
    "body": "<div id=\"comment:29\"></div>\n\nBranch pushed to git repo; I updated commit sha1. New commits:\n<table><tr><td><a href=\"https://github.com/sagemath/sagetrac-mirror/commit/07f054b3e21eac33ee027e8fac148bcefb77fcb5\"><code>07f054b</code></a></td><td><code>Trac 20731: fix doc + simpler code</code></td></tr><tr><td><a href=\"https://github.com/sagemath/sagetrac-mirror/commit/1c87b3cbb27e4f3489707ec30757dd5caa79df73\"><code>1c87b3c</code></a></td><td><code>Trac 20731: remove mpq_sub_z and mpq_div_z</code></td></tr></table>\n",
    "created_at": "2016-06-04T11:20:26Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20731",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20731#issuecomment-300225",
    "user": "https://github.com/sagetrac-git"
}

Branch pushed to git repo; I updated commit sha1. New commits:

`07f054b`	`Trac 20731: fix doc + simpler code`
`1c87b3c`	`Trac 20731: remove mpq_sub_z and mpq_div_z`

archive/issue_comments_300226.json:

{
    "body": "Changed commit from **[`675f8e6`](https://github.com/sagemath/sagetrac-mirror/commit/675f8e617bdcc66e718d18089938b8d1458d0724)** to **[`1c87b3c`](https://github.com/sagemath/sagetrac-mirror/commit/1c87b3cbb27e4f3489707ec30757dd5caa79df73)**",
    "created_at": "2016-06-04T11:20:26Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20731",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20731#issuecomment-300226",
    "user": "https://github.com/sagetrac-git"
}

Changed commit from 675f8e6 to 1c87b3c

archive/issue_events_289326.json:

{
    "actor": "https://github.com/videlec",
    "created_at": "2016-06-04T11:26:00Z",
    "event": "unlabeled",
    "issue": "https://github.com/sagemath/sage/issues/20731",
    "label": "https://github.com/sagemath/sage/labels/needs%20work",
    "label_color": "ffff00",
    "label_name": "needs work",
    "label_text_color": "ffffff",
    "type": "issue_event",
    "url": "https://github.com/sagemath/sage/issues/20731#event-289326"
}

archive/issue_events_289327.json:

{
    "actor": "https://github.com/videlec",
    "created_at": "2016-06-04T11:26:00Z",
    "event": "labeled",
    "issue": "https://github.com/sagemath/sage/issues/20731",
    "label": "https://github.com/sagemath/sage/labels/needs%20review",
    "label_color": "7fff00",
    "label_name": "needs review",
    "label_text_color": "ffffff",
    "type": "issue_event",
    "url": "https://github.com/sagemath/sage/issues/20731#event-289327"
}

archive/issue_comments_300227.json:

{
    "body": "<div id=\"comment:31\" align=\"right\">comment:31</div>\n\nCuriously `Integer + Rational` is faster than `Rational + Integer`\n\n```\nsage: z = 2\nsage: q = QQ((-1,5))\nsage: %timeit z + q\n1000000 loops, best of 3: 251 ns per loop\nsage: %timeit q + z\n1000000 loops, best of 3: 323 ns per loop\n```",
    "created_at": "2016-06-04T11:27:42Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20731",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20731#issuecomment-300227",
    "user": "https://github.com/videlec"
}

comment:31

Curiously Integer + Rational is faster than Rational + Integer

sage: z = 2
sage: q = QQ((-1,5))
sage: %timeit z + q
1000000 loops, best of 3: 251 ns per loop
sage: %timeit q + z
1000000 loops, best of 3: 323 ns per loop

archive/issue_events_289328.json:

{
    "actor": "https://github.com/tscrim",
    "created_at": "2016-06-25T04:17:48Z",
    "event": "unlabeled",
    "issue": "https://github.com/sagemath/sage/issues/20731",
    "label": "https://github.com/sagemath/sage/labels/needs%20review",
    "label_color": "7fff00",
    "label_name": "needs review",
    "label_text_color": "ffffff",
    "type": "issue_event",
    "url": "https://github.com/sagemath/sage/issues/20731#event-289328"
}

archive/issue_events_289329.json:

{
    "actor": "https://github.com/tscrim",
    "created_at": "2016-06-25T04:17:48Z",
    "event": "labeled",
    "issue": "https://github.com/sagemath/sage/issues/20731",
    "label": "https://github.com/sagemath/sage/labels/needs%20work",
    "label_color": "ffff00",
    "label_name": "needs work",
    "label_text_color": "ffffff",
    "type": "issue_event",
    "url": "https://github.com/sagemath/sage/issues/20731#event-289329"
}

archive/issue_comments_300228.json:

{
    "body": "Changed keywords from **sd74** to **days74**",
    "created_at": "2016-06-25T04:17:48Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20731",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20731#issuecomment-300228",
    "user": "https://github.com/tscrim"
}

Changed keywords from sd74 to days74

archive/issue_comments_300229.json:

{
    "body": "<div id=\"comment:32\" align=\"right\">comment:32</div>\n\nI'm not quite sure without looking through it. It might be something at a lower level than Sage.\n\nHowever, this needs rebasing.",
    "created_at": "2016-06-25T04:17:48Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20731",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20731#issuecomment-300229",
    "user": "https://github.com/tscrim"
}

comment:32

I'm not quite sure without looking through it. It might be something at a lower level than Sage.

However, this needs rebasing.

archive/issue_comments_300230.json:

{
    "body": "<div id=\"comment:33\"></div>\n\nBranch pushed to git repo; I updated commit sha1. New commits:\n<table><tr><td><a href=\"https://github.com/sagemath/sagetrac-mirror/commit/1a93a12c39522dbe60b958757e2e3922aa9c23ed\"><code>1a93a12</code></a></td><td><code>Merge branch 'develop' into int_rat_coercion</code></td></tr></table>\n",
    "created_at": "2016-06-26T20:53:01Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20731",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20731#issuecomment-300230",
    "user": "https://github.com/sagetrac-git"
}

Branch pushed to git repo; I updated commit sha1. New commits:

1a93a12 Merge branch 'develop' into int_rat_coercion

archive/issue_comments_300231.json:

{
    "body": "Changed commit from **[`1c87b3c`](https://github.com/sagemath/sagetrac-mirror/commit/1c87b3cbb27e4f3489707ec30757dd5caa79df73)** to **[`1a93a12`](https://github.com/sagemath/sagetrac-mirror/commit/1a93a12c39522dbe60b958757e2e3922aa9c23ed)**",
    "created_at": "2016-06-26T20:53:01Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20731",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20731#issuecomment-300231",
    "user": "https://github.com/sagetrac-git"
}

Changed commit from 1c87b3c to 1a93a12

archive/issue_events_289330.json:

{
    "actor": "https://github.com/videlec",
    "created_at": "2016-06-26T20:56:32Z",
    "event": "unlabeled",
    "issue": "https://github.com/sagemath/sage/issues/20731",
    "label": "https://github.com/sagemath/sage/labels/needs%20work",
    "label_color": "ffff00",
    "label_name": "needs work",
    "label_text_color": "ffffff",
    "type": "issue_event",
    "url": "https://github.com/sagemath/sage/issues/20731#event-289330"
}

archive/issue_events_289331.json:

{
    "actor": "https://github.com/videlec",
    "created_at": "2016-06-26T20:56:32Z",
    "event": "labeled",
    "issue": "https://github.com/sagemath/sage/issues/20731",
    "label": "https://github.com/sagemath/sage/labels/needs%20review",
    "label_color": "7fff00",
    "label_name": "needs review",
    "label_text_color": "ffffff",
    "type": "issue_event",
    "url": "https://github.com/sagemath/sage/issues/20731#event-289331"
}

archive/issue_comments_300232.json:

{
    "body": "<div id=\"comment:34\" align=\"right\">comment:34</div>\n\nReplying to [@tscrim](#comment%3A32):\n> I'm not quite sure without looking through it. It might be something at a lower level than Sage.\n\nI really do not understand this comment.\n\n> However, this needs rebasing.\n\nThis I understood. Done!",
    "created_at": "2016-06-26T20:56:32Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20731",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20731#issuecomment-300232",
    "user": "https://github.com/videlec"
}

comment:34

Replying to @tscrim:

I'm not quite sure without looking through it. It might be something at a lower level than Sage.

I really do not understand this comment.

However, this needs rebasing.

This I understood. Done!

archive/issue_comments_300233.json:

{
    "body": "<div id=\"comment:35\" align=\"right\">comment:35</div>\n\nReplying to [@videlec](#comment%3A34):\n> Replying to [@tscrim](#comment%3A32):\n> > I'm not quite sure without looking through it. It might be something at a lower level than Sage.\n> \n> \n> I really do not understand this comment.\n\nThis was in reference to [comment:31](#comment%3A31), where I was thinking the issue with the time difference might be something in mpz instead of something we are doing in Sage. It might be better to take the faster version and utilize that addition/multiplication is commutative to get that extra little bit of speed.",
    "created_at": "2016-06-26T21:29:30Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20731",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20731#issuecomment-300233",
    "user": "https://github.com/tscrim"
}

comment:35

Replying to @videlec:

Replying to @tscrim:

I'm not quite sure without looking through it. It might be something at a lower level than Sage.

I really do not understand this comment.

This was in reference to comment:31, where I was thinking the issue with the time difference might be something in mpz instead of something we are doing in Sage. It might be better to take the faster version and utilize that addition/multiplication is commutative to get that extra little bit of speed.

archive/issue_comments_300234.json:

{
    "body": "<div id=\"comment:36\" align=\"right\">comment:36</div>\n\nReplying to [@tscrim](#comment%3A35):\n> Replying to [@videlec](#comment%3A34):\n> > Replying to [@tscrim](#comment%3A32):\n> > > I'm not quite sure without looking through it. It might be something at a lower level than Sage.\n> > \n> > \n> > I really do not understand this comment.\n> \n> \n> This was in reference to [comment:31](#comment%3A31), where I was thinking the issue with the time difference might be something in mpz instead of something we are doing in Sage. It might be better to take the faster version and utilize that addition/multiplication is commutative to get that extra little bit of speed.\n\nThere is no fundamental difference between `z+q` and `q+z`. The code is copy paste calling the very same functions. [comment:31](#comment%3A31) is just something that I do not understand.",
    "created_at": "2016-06-27T05:45:47Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20731",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20731#issuecomment-300234",
    "user": "https://github.com/videlec"
}

comment:36

Replying to @tscrim:

Replying to @videlec:

Replying to @tscrim:

I'm not quite sure without looking through it. It might be something at a lower level than Sage.

I really do not understand this comment.

This was in reference to comment:31, where I was thinking the issue with the time difference might be something in mpz instead of something we are doing in Sage. It might be better to take the faster version and utilize that addition/multiplication is commutative to get that extra little bit of speed.

There is no fundamental difference between z+q and q+z. The code is copy paste calling the very same functions. comment:31 is just something that I do not understand.

archive/issue_comments_300235.json:

{
    "body": "Reviewer: **Jeroen Demeyer, Travis Scrimshaw**",
    "created_at": "2016-06-27T13:58:04Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20731",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20731#issuecomment-300235",
    "user": "https://github.com/tscrim"
}

Reviewer: Jeroen Demeyer, Travis Scrimshaw

archive/issue_comments_300236.json:

{
    "body": "<div id=\"comment:37\" align=\"right\">comment:37</div>\n\nAh, I see it now. I guess the only difference is checking `isinstance` for `Integer` is longer than that of `Rational` because of a longer MRO:\n\n```\nsage: p = 2\nsage: p.__class__.__mro__\n(<type 'sage.rings.integer.Integer'>,\n <type 'sage.structure.element.EuclideanDomainElement'>,\n <type 'sage.structure.element.PrincipalIdealDomainElement'>,\n <type 'sage.structure.element.DedekindDomainElement'>,\n <type 'sage.structure.element.IntegralDomainElement'>,\n <type 'sage.structure.element.CommutativeRingElement'>,\n <type 'sage.structure.element.RingElement'>,\n <type 'sage.structure.element.ModuleElement'>,\n <type 'sage.structure.element.Element'>,\n <type 'sage.structure.sage_object.SageObject'>,\n <type 'object'>)\nsage: q = 2/3\nsage: q.__class__.__mro__\n(<type 'sage.rings.rational.Rational'>,\n <type 'sage.structure.element.FieldElement'>,\n <type 'sage.structure.element.CommutativeRingElement'>,\n <type 'sage.structure.element.RingElement'>,\n <type 'sage.structure.element.ModuleElement'>,\n <type 'sage.structure.element.Element'>,\n <type 'sage.structure.sage_object.SageObject'>,\n <type 'object'>)\n```\n\nI'm happy with this ticket as-is. Jeroen, any more comments?",
    "created_at": "2016-06-27T13:58:04Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20731",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20731#issuecomment-300236",
    "user": "https://github.com/tscrim"
}

comment:37

Ah, I see it now. I guess the only difference is checking isinstance for Integer is longer than that of Rational because of a longer MRO:

sage: p = 2
sage: p.__class__.__mro__
(<type 'sage.rings.integer.Integer'>,
 <type 'sage.structure.element.EuclideanDomainElement'>,
 <type 'sage.structure.element.PrincipalIdealDomainElement'>,
 <type 'sage.structure.element.DedekindDomainElement'>,
 <type 'sage.structure.element.IntegralDomainElement'>,
 <type 'sage.structure.element.CommutativeRingElement'>,
 <type 'sage.structure.element.RingElement'>,
 <type 'sage.structure.element.ModuleElement'>,
 <type 'sage.structure.element.Element'>,
 <type 'sage.structure.sage_object.SageObject'>,
 <type 'object'>)
sage: q = 2/3
sage: q.__class__.__mro__
(<type 'sage.rings.rational.Rational'>,
 <type 'sage.structure.element.FieldElement'>,
 <type 'sage.structure.element.CommutativeRingElement'>,
 <type 'sage.structure.element.RingElement'>,
 <type 'sage.structure.element.ModuleElement'>,
 <type 'sage.structure.element.Element'>,
 <type 'sage.structure.sage_object.SageObject'>,
 <type 'object'>)

I'm happy with this ticket as-is. Jeroen, any more comments?

archive/issue_comments_300237.json:

{
    "body": "<div id=\"comment:38\" align=\"right\">comment:38</div>\n\nOne way to shortcut the `isinstance(right, Rational)` is to instead do `type(right) is Rational`. That would actually be the fastest. (And it will still work with an element that inherits from `Rational` since `_add_`, `_mul_` etc are properly implemented.) What do you think?",
    "created_at": "2016-07-08T20:37:05Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20731",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20731#issuecomment-300237",
    "user": "https://github.com/videlec"
}

comment:38

One way to shortcut the isinstance(right, Rational) is to instead do type(right) is Rational. That would actually be the fastest. (And it will still work with an element that inherits from Rational since _add_, _mul_ etc are properly implemented.) What do you think?

archive/issue_comments_300238.json:

{
    "body": "<div id=\"comment:39\" align=\"right\">comment:39</div>\n\nI was originally thinking that we should keep it as `isinstance`, but since we explicitly construct a new `Rational` (and I don't think it could be made to work with subtypes [easily]), I think we should only test against `Rational`. Plus those extra little nanoseconds can matter at this level. Subclasses can just fall into the coercion framework.",
    "created_at": "2016-07-09T19:19:34Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20731",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20731#issuecomment-300238",
    "user": "https://github.com/tscrim"
}

comment:39

I was originally thinking that we should keep it as isinstance, but since we explicitly construct a new Rational (and I don't think it could be made to work with subtypes [easily]), I think we should only test against Rational. Plus those extra little nanoseconds can matter at this level. Subclasses can just fall into the coercion framework.

archive/issue_comments_300239.json:

{
    "body": "<div id=\"comment:40\" align=\"right\">comment:40</div>\n\nhum... with `isinstance(X,Y)`\n\n```\nsage: %timeit z+q                                                              \n1000000 loops, best of 3: 242 ns per loop\nsage: %timeit q+z                                                              \n1000000 loops, best of 3: 306 ns per loop\n```\nwith `type(X) is Y`\n\n```\nsage: %timeit z+q\n1000000 loops, best of 3: 243 ns per loop                                      \nsage: %timeit q+z\n1000000 loops, best of 3: 334 ns per loop\n```",
    "created_at": "2016-07-09T20:35:46Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20731",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20731#issuecomment-300239",
    "user": "https://github.com/videlec"
}

comment:40

hum... with isinstance(X,Y)

sage: %timeit z+q                                                              
1000000 loops, best of 3: 242 ns per loop
sage: %timeit q+z                                                              
1000000 loops, best of 3: 306 ns per loop

with type(X) is Y

sage: %timeit z+q
1000000 loops, best of 3: 243 ns per loop                                      
sage: %timeit q+z
1000000 loops, best of 3: 334 ns per loop

archive/issue_comments_300240.json:

{
    "body": "<div id=\"comment:41\" align=\"right\">comment:41</div>\n\nIndependent time testing with a Cython class (without inheritance)\n\n|  code | relative time |\n|:-------:|:---------------:|\n| `type(a) == A`      | 10500000ns|\n| `isinstance(a,A)`   |   559000ns|\n| `type(a) is A`      |       55ns|\n\nI guess something else is happening with `z+q` versus `q+z`...",
    "created_at": "2016-07-09T20:50:47Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20731",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20731#issuecomment-300240",
    "user": "https://github.com/videlec"
}

comment:41

Independent time testing with a Cython class (without inheritance)

code	relative time
`type(a) == A`	10500000ns
`isinstance(a,A)`	559000ns
`type(a) is A`	55ns

I guess something else is happening with z+q versus q+z...

archive/issue_comments_300241.json:

{
    "body": "<div id=\"comment:42\"></div>\n\nBranch pushed to git repo; I updated commit sha1. New commits:\n<table><tr><td><a href=\"https://github.com/sagemath/sagetrac-mirror/commit/dd2defb5ddf707d2dc5a84419a41fdbcd5c75a30\"><code>dd2defb</code></a></td><td><code>Trac 20731: isinstance(X,Y) -> type(X) is Y</code></td></tr></table>\n",
    "created_at": "2016-07-09T20:51:59Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20731",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20731#issuecomment-300241",
    "user": "https://github.com/sagetrac-git"
}

Branch pushed to git repo; I updated commit sha1. New commits:

dd2defb Trac 20731: isinstance(X,Y) -> type(X) is Y

archive/issue_comments_300242.json:

{
    "body": "Changed commit from **[`1a93a12`](https://github.com/sagemath/sagetrac-mirror/commit/1a93a12c39522dbe60b958757e2e3922aa9c23ed)** to **[`dd2defb`](https://github.com/sagemath/sagetrac-mirror/commit/dd2defb5ddf707d2dc5a84419a41fdbcd5c75a30)**",
    "created_at": "2016-07-09T20:51:59Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20731",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20731#issuecomment-300242",
    "user": "https://github.com/sagetrac-git"
}

Changed commit from 1a93a12 to dd2defb

archive/issue_events_289332.json:

{
    "actor": "https://github.com/tscrim",
    "created_at": "2016-07-10T18:01:19Z",
    "event": "unlabeled",
    "issue": "https://github.com/sagemath/sage/issues/20731",
    "label": "https://github.com/sagemath/sage/labels/needs%20review",
    "label_color": "7fff00",
    "label_name": "needs review",
    "label_text_color": "ffffff",
    "type": "issue_event",
    "url": "https://github.com/sagemath/sage/issues/20731#event-289332"
}

archive/issue_events_289333.json:

{
    "actor": "https://github.com/tscrim",
    "created_at": "2016-07-10T18:01:19Z",
    "event": "labeled",
    "issue": "https://github.com/sagemath/sage/issues/20731",
    "label": "https://github.com/sagemath/sage/labels/positive%20review",
    "label_color": "dfffc0",
    "label_name": "positive review",
    "label_text_color": "ffffff",
    "type": "issue_event",
    "url": "https://github.com/sagemath/sage/issues/20731#event-289333"
}

archive/issue_comments_300243.json:

{
    "body": "<div id=\"comment:43\" align=\"right\">comment:43</div>\n\nThat really suggests that it has to do with the MRO length (comment:37), but I am somewhat surprised that does matter. Well, this is a definite improvement and should get into 7.3.",
    "created_at": "2016-07-10T18:01:19Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20731",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20731#issuecomment-300243",
    "user": "https://github.com/tscrim"
}

comment:43

That really suggests that it has to do with the MRO length (comment:37), but I am somewhat surprised that does matter. Well, this is a definite improvement and should get into 7.3.

archive/issue_events_289334.json:

{
    "actor": "https://github.com/vbraun",
    "created_at": "2016-07-10T21:44:47Z",
    "event": "unlabeled",
    "issue": "https://github.com/sagemath/sage/issues/20731",
    "label": "https://github.com/sagemath/sage/labels/positive%20review",
    "label_color": "dfffc0",
    "label_name": "positive review",
    "label_text_color": "ffffff",
    "type": "issue_event",
    "url": "https://github.com/sagemath/sage/issues/20731#event-289334"
}

archive/issue_events_289335.json:

{
    "actor": "https://github.com/vbraun",
    "created_at": "2016-07-10T21:44:47Z",
    "event": "labeled",
    "issue": "https://github.com/sagemath/sage/issues/20731",
    "label": "https://github.com/sagemath/sage/labels/needs%20work",
    "label_color": "ffff00",
    "label_name": "needs work",
    "label_text_color": "ffffff",
    "type": "issue_event",
    "url": "https://github.com/sagemath/sage/issues/20731#event-289335"
}

archive/issue_comments_300244.json:

{
    "body": "<div id=\"comment:44\" align=\"right\">comment:44</div>\n\n\n```\nsage -t --long src/sage/repl/interpreter.py\n**********************************************************************\nFile \"src/sage/repl/interpreter.py\", line 77, in sage.repl.interpreter\nFailed example:\n    shell.run_cell('1/0')\nExpected:\n    ---------------------------------------------------------------------------\n    ZeroDivisionError                         Traceback (most recent call last)\n    <ipython-input-...> in <module>()\n    ----> 1 Integer(1)/Integer(0)\n    <BLANKLINE>\n    .../src/sage/rings/integer.pyx in sage.rings.integer.Integer.__div__ (build/cythonized/sage/rings/integer.c:...)()\n       ...          if type(left) is type(right):\n       ...              if mpz_sgn((<Integer>right).value) == 0:\n    -> ...                  raise ZeroDivisionError(\"rational division by zero\")\n       ...              x = <Rational> Rational.__new__(Rational)\n       ...              mpq_div_zz(x.value, (<Integer>left).value, (<Integer>right).value)\n    <BLANKLINE>\n    ZeroDivisionError: rational division by zero\nGot:\n    ---------------------------------------------------------------------------\n    ZeroDivisionError                         Traceback (most recent call last)\n    <ipython-input-1-6f88eab09598> in <module>()\n    ----> 1 Integer(1)/Integer(0)\n    <BLANKLINE>\n    /home/buildslave-sage/slave/sage_git/build/src/sage/rings/integer.pyx in sage.rings.integer.Integer.__div__ (/home/buildslave-sage/slave/sage_git/build/src/build/cythonized/sage/rings/integer.c:12882)()\n       1841         if type(left) is type(right):\n       1842             if mpz_sgn((<Integer>right).value) == 0:\n    -> 1843                 raise ZeroDivisionError(\"rational division by zero\")\n       1844             x = <Rational> Rational.__new__(Rational)\n       1845             mpq_div_zz(x.value, (<Integer>left).value, (<Integer>right).value)\n    <BLANKLINE>\n    ZeroDivisionError: rational division by zero\n**********************************************************************\n1 item had failures:\n   1 of   5 in sage.repl.interpreter\n    [133 tests, 1 failure, 3.60 s]\n```",
    "created_at": "2016-07-10T21:44:47Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20731",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20731#issuecomment-300244",
    "user": "https://github.com/vbraun"
}

comment:44

sage -t --long src/sage/repl/interpreter.py
**********************************************************************
File "src/sage/repl/interpreter.py", line 77, in sage.repl.interpreter
Failed example:
    shell.run_cell('1/0')
Expected:
    ---------------------------------------------------------------------------
    ZeroDivisionError                         Traceback (most recent call last)
    <ipython-input-...> in <module>()
    ----> 1 Integer(1)/Integer(0)
    <BLANKLINE>
    .../src/sage/rings/integer.pyx in sage.rings.integer.Integer.__div__ (build/cythonized/sage/rings/integer.c:...)()
       ...          if type(left) is type(right):
       ...              if mpz_sgn((<Integer>right).value) == 0:
    -> ...                  raise ZeroDivisionError("rational division by zero")
       ...              x = <Rational> Rational.__new__(Rational)
       ...              mpq_div_zz(x.value, (<Integer>left).value, (<Integer>right).value)
    <BLANKLINE>
    ZeroDivisionError: rational division by zero
Got:
    ---------------------------------------------------------------------------
    ZeroDivisionError                         Traceback (most recent call last)
    <ipython-input-1-6f88eab09598> in <module>()
    ----> 1 Integer(1)/Integer(0)
    <BLANKLINE>
    /home/buildslave-sage/slave/sage_git/build/src/sage/rings/integer.pyx in sage.rings.integer.Integer.__div__ (/home/buildslave-sage/slave/sage_git/build/src/build/cythonized/sage/rings/integer.c:12882)()
       1841         if type(left) is type(right):
       1842             if mpz_sgn((<Integer>right).value) == 0:
    -> 1843                 raise ZeroDivisionError("rational division by zero")
       1844             x = <Rational> Rational.__new__(Rational)
       1845             mpq_div_zz(x.value, (<Integer>left).value, (<Integer>right).value)
    <BLANKLINE>
    ZeroDivisionError: rational division by zero
**********************************************************************
1 item had failures:
   1 of   5 in sage.repl.interpreter
    [133 tests, 1 failure, 3.60 s]

archive/issue_comments_300245.json:

{
    "body": "<div id=\"comment:45\" align=\"right\">comment:45</div>\n\nDear Volker,\n\nI am not able to reproduce this... could you tell us on which kind of configuration you obtained that?\n\nVincent",
    "created_at": "2016-07-11T16:15:31Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20731",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20731#issuecomment-300245",
    "user": "https://github.com/videlec"
}

comment:45

Dear Volker,

I am not able to reproduce this... could you tell us on which kind of configuration you obtained that?

Vincent

archive/issue_comments_300246.json:

{
    "body": "<div id=\"comment:46\" align=\"right\">comment:46</div>\n\nI think the test is just fragile:\n\n```\n    .../src/sage/rings/integer.pyx in sage.rings.integer.Integer.__div__ (build/cythonized/sage/rings/integer.c:...)()\n```\nvs the actual output:\n\n```\n    /home/buildslave-sage/slave/sage_git/build/src/sage/rings/integer.pyx in sage.rings.integer.Integer.__div__ (/home/buildslave-sage/slave/sage_git/build/src/build/cythonized/sage/rings/integer.c:12882)()\n```\nNote the path is `.../build/src/...` and compare with the previous doctest output format.",
    "created_at": "2016-07-15T20:00:53Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20731",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20731#issuecomment-300246",
    "user": "https://github.com/tscrim"
}

comment:46

I think the test is just fragile:

    .../src/sage/rings/integer.pyx in sage.rings.integer.Integer.__div__ (build/cythonized/sage/rings/integer.c:...)()

vs the actual output:

    /home/buildslave-sage/slave/sage_git/build/src/sage/rings/integer.pyx in sage.rings.integer.Integer.__div__ (/home/buildslave-sage/slave/sage_git/build/src/build/cythonized/sage/rings/integer.c:12882)()

Note the path is .../build/src/... and compare with the previous doctest output format.

archive/issue_comments_300247.json:

{
    "body": "<div id=\"comment:47\"></div>\n\nBranch pushed to git repo; I updated commit sha1. New commits:\n<table><tr><td><a href=\"https://github.com/sagemath/sagetrac-mirror/commit/d535aee38e629195fa6319a8a1d1d2efaecebec6\"><code>d535aee</code></a></td><td><code>Trac 20731: fix doctest</code></td></tr></table>\n",
    "created_at": "2016-07-22T22:41:34Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20731",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20731#issuecomment-300247",
    "user": "https://github.com/sagetrac-git"
}

Branch pushed to git repo; I updated commit sha1. New commits:

d535aee Trac 20731: fix doctest

archive/issue_comments_300248.json:

{
    "body": "Changed commit from **[`dd2defb`](https://github.com/sagemath/sagetrac-mirror/commit/dd2defb5ddf707d2dc5a84419a41fdbcd5c75a30)** to **[`d535aee`](https://github.com/sagemath/sagetrac-mirror/commit/d535aee38e629195fa6319a8a1d1d2efaecebec6)**",
    "created_at": "2016-07-22T22:41:34Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20731",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20731#issuecomment-300248",
    "user": "https://github.com/sagetrac-git"
}

Changed commit from dd2defb to d535aee

archive/issue_events_289336.json:

{
    "actor": "https://github.com/videlec",
    "created_at": "2016-07-22T22:42:10Z",
    "event": "unlabeled",
    "issue": "https://github.com/sagemath/sage/issues/20731",
    "label": "https://github.com/sagemath/sage/labels/needs%20work",
    "label_color": "ffff00",
    "label_name": "needs work",
    "label_text_color": "ffffff",
    "type": "issue_event",
    "url": "https://github.com/sagemath/sage/issues/20731#event-289336"
}

archive/issue_events_289337.json:

{
    "actor": "https://github.com/videlec",
    "created_at": "2016-07-22T22:42:10Z",
    "event": "labeled",
    "issue": "https://github.com/sagemath/sage/issues/20731",
    "label": "https://github.com/sagemath/sage/labels/needs%20review",
    "label_color": "7fff00",
    "label_name": "needs review",
    "label_text_color": "ffffff",
    "type": "issue_event",
    "url": "https://github.com/sagemath/sage/issues/20731#event-289337"
}

archive/issue_events_289338.json:

{
    "actor": "https://github.com/tscrim",
    "created_at": "2016-07-22T22:47:47Z",
    "event": "unlabeled",
    "issue": "https://github.com/sagemath/sage/issues/20731",
    "label": "https://github.com/sagemath/sage/labels/needs%20review",
    "label_color": "7fff00",
    "label_name": "needs review",
    "label_text_color": "ffffff",
    "type": "issue_event",
    "url": "https://github.com/sagemath/sage/issues/20731#event-289338"
}

archive/issue_events_289339.json:

{
    "actor": "https://github.com/tscrim",
    "created_at": "2016-07-22T22:47:47Z",
    "event": "labeled",
    "issue": "https://github.com/sagemath/sage/issues/20731",
    "label": "https://github.com/sagemath/sage/labels/positive%20review",
    "label_color": "dfffc0",
    "label_name": "positive review",
    "label_text_color": "ffffff",
    "type": "issue_event",
    "url": "https://github.com/sagemath/sage/issues/20731#event-289339"
}

archive/issue_events_289340.json:

{
    "actor": "https://github.com/vbraun",
    "created_at": "2016-07-23T18:38:25Z",
    "event": "unlabeled",
    "issue": "https://github.com/sagemath/sage/issues/20731",
    "label": "https://github.com/sagemath/sage/labels/positive%20review",
    "label_color": "dfffc0",
    "label_name": "positive review",
    "label_text_color": "ffffff",
    "type": "issue_event",
    "url": "https://github.com/sagemath/sage/issues/20731#event-289340"
}

archive/issue_events_289341.json:

{
    "actor": "https://github.com/vbraun",
    "commit_id": "38381dffbdc9e9b4755c937bc1761b90bb8a5016",
    "commit_repository": "https://github.com/sagemath/sage",
    "created_at": "2016-07-23T18:38:25Z",
    "event": "closed",
    "issue": "https://github.com/sagemath/sage/issues/20731",
    "type": "issue_event",
    "url": "https://github.com/sagemath/sage/issues/20731#event-289341"
}

archive/issue_comments_300249.json:

{
    "body": "Changed branch from **[u/vdelecroix/20731](https://github.com/sagemath/sagetrac-mirror/tree/u/vdelecroix/20731)** to **[`d535aee`](https://github.com/sagemath/sagetrac-mirror/commit/d535aee38e629195fa6319a8a1d1d2efaecebec6)**",
    "created_at": "2016-07-23T18:38:25Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20731",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20731#issuecomment-300249",
    "user": "https://github.com/vbraun"
}

Changed branch from u/vdelecroix/20731 to d535aee

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Files

20731.md

20731.md

Issue 20731: shortcut coercion for Integer-Rational operations

Files

20731.md

Latest commit

History

20731.md

File metadata and controls

Issue 20731: shortcut coercion for Integer-Rational operations