Issue 20086: rational powers in ZZ[X] and QQ[X]

archive/issues_019849.json:

{
    "assignees": [],
    "body": "<div id=\"comment:0\"></div>\n\nUntil now,\n\n```\nsage: R.<x> = ZZ[]\nsage: R(1)^(1/2)\nTraceback (most recent call last):\n...\nTypeError: rational is not an integer\n```\nbecause only integer exponents were allowed for polynomials.\n\nImplement arbitrary powers of constant polynomials by handing over to the rational field.\n\nThis was originally observed in the asymptotic ring:\n\n```\nsage: P.<R> = QQ[]\nsage: A.<Z> = AsymptoticRing('T^QQ', P)\nsage: sqrt(Z)\nTraceback (most recent call last):\n...\nArithmeticError: Cannot take T to the exponent 1/2 in Exact Term Monoid T^QQ\nwith coefficients in Univariate Polynomial Ring in R over Rational Field\nsince its coefficient 1 cannot be taken to this exponent.\n> *previous* TypeError: rational is not an integer\n```\n\nfollow up: #20571\n\nCC:  @behackl @dkrenn\n\nComponent: **basic arithmetic**\n\nAuthor: **Clemens Heuberger, Vincent Delecroix, Benjamin Hackl**\n\nBranch/Commit: **[`5fc1027`](https://github.com/sagemath/sagetrac-mirror/commit/5fc1027e43d601a791568b22f12ec70957c47519)**\n\nReviewer: **Benjamin Hackl, Vincent Delecroix**\n\n_Issue created by migration from https://trac.sagemath.org/ticket/20086_\n\n",
    "closed_at": "2016-05-17T07:16:35Z",
    "created_at": "2016-02-19T12:52:04Z",
    "labels": [
        "https://github.com/sagemath/sage/labels/c%3A%20basic%20arithmetic",
        "https://github.com/sagemath/sage/labels/p%3A%20major%20/%203",
        "https://github.com/sagemath/sage/labels/bug"
    ],
    "milestone": "https://github.com/sagemath/sage/milestones/sage-7.2",
    "reactions": [],
    "repository": "https://github.com/sagemath/sage",
    "title": "rational powers in ZZ[X] and QQ[X]",
    "type": "issue",
    "updated_at": "2016-05-17T07:16:35Z",
    "url": "https://github.com/sagemath/sage/issues/20086",
    "user": "https://github.com/cheuberg"
}

Until now,

sage: R.<x> = ZZ[]
sage: R(1)^(1/2)
Traceback (most recent call last):
...
TypeError: rational is not an integer

because only integer exponents were allowed for polynomials.

Implement arbitrary powers of constant polynomials by handing over to the rational field.

This was originally observed in the asymptotic ring:

sage: P.<R> = QQ[]
sage: A.<Z> = AsymptoticRing('T^QQ', P)
sage: sqrt(Z)
Traceback (most recent call last):
...
ArithmeticError: Cannot take T to the exponent 1/2 in Exact Term Monoid T^QQ
with coefficients in Univariate Polynomial Ring in R over Rational Field
since its coefficient 1 cannot be taken to this exponent.
> *previous* TypeError: rational is not an integer

follow up: #20571

CC: @behackl @dkrenn

Component: basic arithmetic

Author: Clemens Heuberger, Vincent Delecroix, Benjamin Hackl

Branch/Commit: 5fc1027

Reviewer: Benjamin Hackl, Vincent Delecroix

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

archive/issue_events_281131.json:

{
    "actor": "https://github.com/cheuberg",
    "created_at": "2016-02-19T12:52:04Z",
    "event": "milestoned",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "milestone_number": null,
    "milestone_title": "sage-7.1",
    "type": "issue_event",
    "url": "https://github.com/sagemath/sage/issues/20086#event-281131"
}

archive/issue_events_281132.json:

{
    "actor": "https://github.com/cheuberg",
    "created_at": "2016-02-19T12:52:04Z",
    "event": "labeled",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "label": "https://github.com/sagemath/sage/labels/c%3A%20asymptotic%20expansions",
    "label_color": "0000ff",
    "label_name": "c: asymptotic expansions",
    "label_text_color": "ffffff",
    "type": "issue_event",
    "url": "https://github.com/sagemath/sage/issues/20086#event-281132"
}

archive/issue_events_281133.json:

{
    "actor": "https://github.com/cheuberg",
    "created_at": "2016-02-19T12:52:04Z",
    "event": "labeled",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "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/20086#event-281133"
}

archive/issue_events_281134.json:

{
    "actor": "https://github.com/cheuberg",
    "created_at": "2016-02-19T12:52:04Z",
    "event": "labeled",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "label": "https://github.com/sagemath/sage/labels/bug",
    "label_color": "d73a4a",
    "label_name": "bug",
    "label_text_color": "ffffff",
    "type": "issue_event",
    "url": "https://github.com/sagemath/sage/issues/20086#event-281134"
}

archive/issue_comments_287855.json:

{
    "body": "Branch: **[u/cheuberg/polynomials/power](https://github.com/sagemath/sagetrac-mirror/tree/u/cheuberg/polynomials/power)**",
    "created_at": "2016-02-19T13:07:16Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20086#issuecomment-287855",
    "user": "https://github.com/cheuberg"
}

Branch: u/cheuberg/polynomials/power

archive/issue_comments_287856.json:

{
    "body": "Commit: **[`cc49098`](https://github.com/sagemath/sagetrac-mirror/commit/cc49098e1fc111e63d344b6c2f914d0d3e5e463b)**",
    "created_at": "2016-02-19T13:19:09Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20086#issuecomment-287856",
    "user": "https://github.com/sagetrac-git"
}

Commit: cc49098

archive/issue_comments_287857.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/cc49098e1fc111e63d344b6c2f914d0d3e5e463b\"><code>cc49098</code></a></td><td><code>Trac #20086: powers of constant polynomials</code></td></tr></table>\n",
    "created_at": "2016-02-19T13:19:09Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20086#issuecomment-287857",
    "user": "https://github.com/sagetrac-git"
}

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

cc49098 Trac #20086: powers of constant polynomials

archive/issue_comments_287858.json:

{
    "body": "Description changed:\n``````diff\n--- \n+++ \n@@ -1,3 +1,20 @@\n+Until now,\n+\n+```\n+sage: P.<R> = QQ[]\n+sage: P(1)^(1/2)\n+Traceback (most recent call last):\n+...\n+ArithmeticError: Cannot take T to the exponent 1/2 in Exact Term Monoid T^QQ\n+with coefficients in Univariate Polynomial Ring in R over Rational Field\n+since its coefficient 1 cannot be taken to this exponent.\n+> *previous* TypeError: rational is not an integer\n+```\n+because only integer exponents were allowed for polynomials.\n+\n+Implement arbitrary powers of constant polynomials by handing over to the rational field.\n+\n+This was originally observed in the asymptotic ring:\n \n ```\n sage: P.<R> = QQ[]\n``````\n",
    "created_at": "2016-02-19T13:22:52Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20086#issuecomment-287858",
    "user": "https://github.com/cheuberg"
}

Description changed:

--- 
+++ 
@@ -1,3 +1,20 @@
+Until now,
+
+```
+sage: P.<R> = QQ[]
+sage: P(1)^(1/2)
+Traceback (most recent call last):
+...
+ArithmeticError: Cannot take T to the exponent 1/2 in Exact Term Monoid T^QQ
+with coefficients in Univariate Polynomial Ring in R over Rational Field
+since its coefficient 1 cannot be taken to this exponent.
+> *previous* TypeError: rational is not an integer
+```
+because only integer exponents were allowed for polynomials.
+
+Implement arbitrary powers of constant polynomials by handing over to the rational field.
+
+This was originally observed in the asymptotic ring:
 
 ```
 sage: P.<R> = QQ[]

archive/issue_events_281135.json:

{
    "actor": "https://github.com/cheuberg",
    "created_at": "2016-02-19T13:22:52Z",
    "event": "unlabeled",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "label": "https://github.com/sagemath/sage/labels/c%3A%20asymptotic%20expansions",
    "label_color": "0000ff",
    "label_name": "c: asymptotic expansions",
    "label_text_color": "ffffff",
    "type": "issue_event",
    "url": "https://github.com/sagemath/sage/issues/20086#event-281135"
}

archive/issue_events_281136.json:

{
    "actor": "https://github.com/cheuberg",
    "created_at": "2016-02-19T13:22:52Z",
    "event": "labeled",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "label": "https://github.com/sagemath/sage/labels/c%3A%20basic%20arithmetic",
    "label_color": "0000ff",
    "label_name": "c: basic arithmetic",
    "label_text_color": "ffffff",
    "type": "issue_event",
    "url": "https://github.com/sagemath/sage/issues/20086#event-281136"
}

archive/issue_comments_287859.json:

{
    "body": "Author: **Clemens Heuberger**",
    "created_at": "2016-02-19T13:22:52Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20086#issuecomment-287859",
    "user": "https://github.com/cheuberg"
}

Author: Clemens Heuberger

archive/issue_events_281137.json:

{
    "actor": "https://github.com/cheuberg",
    "created_at": "2016-02-19T13:22:52Z",
    "event": "renamed",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "title_is": "QQ[X]: allow arbitrary powers of constant polynomials",
    "title_was": "Asymptotic ring: fix exponentiation",
    "type": "issue_event",
    "url": "https://github.com/sagemath/sage/issues/20086#event-281137"
}

archive/issue_events_281138.json:

{
    "actor": "https://github.com/cheuberg",
    "created_at": "2016-02-19T13:22:52Z",
    "event": "labeled",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "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/20086#event-281138"
}

archive/issue_comments_287860.json:

{
    "body": "Changed commit from **[`cc49098`](https://github.com/sagemath/sagetrac-mirror/commit/cc49098e1fc111e63d344b6c2f914d0d3e5e463b)** to **[`25fdd0c`](https://github.com/sagemath/sagetrac-mirror/commit/25fdd0c0ceb085d68b2ddf851983a386771d164f)**",
    "created_at": "2016-02-19T15:37:53Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20086#issuecomment-287860",
    "user": "https://github.com/sagetrac-git"
}

Changed commit from cc49098 to 25fdd0c

archive/issue_comments_287861.json:

{
    "body": "<div id=\"comment:4\"></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/25fdd0c0ceb085d68b2ddf851983a386771d164f\"><code>25fdd0c</code></a></td><td><code>Trac #20086: powers of constant polynomials over ZZ</code></td></tr></table>\n",
    "created_at": "2016-02-19T15:37:53Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20086#issuecomment-287861",
    "user": "https://github.com/sagetrac-git"
}

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

25fdd0c Trac #20086: powers of constant polynomials over ZZ

archive/issue_events_281139.json:

{
    "actor": "https://github.com/cheuberg",
    "created_at": "2016-02-19T15:39:05Z",
    "event": "renamed",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "title_is": "ZZ[X], QQ[X]: allow arbitrary powers of constant polynomials",
    "title_was": "QQ[X]: allow arbitrary powers of constant polynomials",
    "type": "issue_event",
    "url": "https://github.com/sagemath/sage/issues/20086#event-281139"
}

archive/issue_comments_287862.json:

{
    "body": "<div id=\"comment:6\" align=\"right\">comment:6</div>\n\nThe fact that for a in QQ, the value of `a^(1/2)` has its parent depending on the actual value of a is a compromise for novice use (in calculus etc.) Normally, one would raise an error if `a^(1/2)` does not lie in QQ. Certainly, promoting to SR is a very bad choice for anything else than novice use.\n\nI see you take the effort of trying to put the result back in the original parent. Don't you think it's better to force that? i.e., raise an error if it doesn't work, rather than give a result back in SR?\n\nThe problem is that if someone is computing with polynomials, it's almost certainly not desired to end up in SR (where things like `QQ['x']['x']` get squashed), and if an error isn't raised, it's very hard to detect that it happened.\n\nAlso, if you're implementing the (partial) map `a:->a^(n/m)` , why not extend it properly? When\n`(QQ['x'](4))^(1/2)` works, why should `QQ['x'](x<sup>2)</sup>(1/2)` fail?",
    "created_at": "2016-02-20T01:56:39Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20086#issuecomment-287862",
    "user": "https://github.com/nbruin"
}

comment:6

The fact that for a in QQ, the value of a^(1/2) has its parent depending on the actual value of a is a compromise for novice use (in calculus etc.) Normally, one would raise an error if a^(1/2) does not lie in QQ. Certainly, promoting to SR is a very bad choice for anything else than novice use.

I see you take the effort of trying to put the result back in the original parent. Don't you think it's better to force that? i.e., raise an error if it doesn't work, rather than give a result back in SR?

The problem is that if someone is computing with polynomials, it's almost certainly not desired to end up in SR (where things like QQ['x']['x'] get squashed), and if an error isn't raised, it's very hard to detect that it happened.

Also, if you're implementing the (partial) map a:->a^(n/m) , why not extend it properly? When (QQ['x'](4))^(1/2) works, why should QQ['x'](x2)(1/2) fail?

archive/issue_comments_287863.json:

{
    "body": "<div id=\"comment:7\"></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/6217beeb2520f5e8c0d1e92881c4d1c9569e9868\"><code>6217bee</code></a></td><td><code>Trac #20085: raise exception instead of moving to SR</code></td></tr></table>\n",
    "created_at": "2016-02-22T17:19:21Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20086#issuecomment-287863",
    "user": "https://github.com/sagetrac-git"
}

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

6217bee Trac #20085: raise exception instead of moving to SR

archive/issue_comments_287864.json:

{
    "body": "Changed commit from **[`25fdd0c`](https://github.com/sagemath/sagetrac-mirror/commit/25fdd0c0ceb085d68b2ddf851983a386771d164f)** to **[`6217bee`](https://github.com/sagemath/sagetrac-mirror/commit/6217beeb2520f5e8c0d1e92881c4d1c9569e9868)**",
    "created_at": "2016-02-22T17:19:21Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20086#issuecomment-287864",
    "user": "https://github.com/sagetrac-git"
}

Changed commit from 25fdd0c to 6217bee

archive/issue_comments_287865.json:

{
    "body": "<div id=\"comment:8\"></div>\n\nBranch pushed to git repo; I updated commit sha1. This was a forced push. New commits:\n<table><tr><td><a href=\"https://github.com/sagemath/sagetrac-mirror/commit/918499cf377a551a5e28ea1d6748ed50860f1afa\"><code>918499c</code></a></td><td><code>Trac #20086: raise exception instead of moving to SR</code></td></tr></table>\n",
    "created_at": "2016-02-22T17:28:49Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20086#issuecomment-287865",
    "user": "https://github.com/sagetrac-git"
}

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

918499c Trac #20086: raise exception instead of moving to SR

archive/issue_comments_287866.json:

{
    "body": "Changed commit from **[`6217bee`](https://github.com/sagemath/sagetrac-mirror/commit/6217beeb2520f5e8c0d1e92881c4d1c9569e9868)** to **[`918499c`](https://github.com/sagemath/sagetrac-mirror/commit/918499cf377a551a5e28ea1d6748ed50860f1afa)**",
    "created_at": "2016-02-22T17:28:49Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20086#issuecomment-287866",
    "user": "https://github.com/sagetrac-git"
}

Changed commit from 6217bee to 918499c

archive/issue_comments_287867.json:

{
    "body": "<div id=\"comment:9\" align=\"right\">comment:9</div>\n\nReplying to [@nbruin](#comment%3A6):\n> The fact that for a in QQ, the value of `a^(1/2)` has its parent depending on the actual value of a is a compromise for novice use (in calculus etc.) Normally, one would raise an error if `a^(1/2)` does not lie in QQ. Certainly, promoting to SR is a very bad choice for anything else than novice use.\n> \n> I see you take the effort of trying to put the result back in the original parent. Don't you think it's better to force that? i.e., raise an error if it doesn't work, rather than give a result back in SR?\n> \n> The problem is that if someone is computing with polynomials, it's almost certainly not desired to end up in SR (where things like `QQ['x']['x']` get squashed), and if an error isn't raised, it's very hard to detect that it happened.\n\nvery valid points, thank you. I now raise an exception.\n\n> \n> Also, if you're implementing the (partial) map `a:->a^(n/m)` , why not extend it properly? When\n> `(QQ['x'](4))^(1/2)` works, why should `QQ['x'](x<sup>2)</sup>(1/2)` fail?\n\nI needed a fix a bug which did bite me somewhere else, see initial description. Of course, it would be nice to have that, too; but I'd prefer to leave that to a follow-up ticket and to have this functionality here soon.",
    "created_at": "2016-02-22T17:33:23Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20086#issuecomment-287867",
    "user": "https://github.com/cheuberg"
}

comment:9

Replying to @nbruin:

The fact that for a in QQ, the value of a^(1/2) has its parent depending on the actual value of a is a compromise for novice use (in calculus etc.) Normally, one would raise an error if a^(1/2) does not lie in QQ. Certainly, promoting to SR is a very bad choice for anything else than novice use.

I see you take the effort of trying to put the result back in the original parent. Don't you think it's better to force that? i.e., raise an error if it doesn't work, rather than give a result back in SR?

The problem is that if someone is computing with polynomials, it's almost certainly not desired to end up in SR (where things like QQ['x']['x'] get squashed), and if an error isn't raised, it's very hard to detect that it happened.

very valid points, thank you. I now raise an exception.

Also, if you're implementing the (partial) map a:->a^(n/m) , why not extend it properly? When (QQ['x'](4))^(1/2) works, why should QQ['x'](x2)(1/2) fail?

I needed a fix a bug which did bite me somewhere else, see initial description. Of course, it would be nice to have that, too; but I'd prefer to leave that to a follow-up ticket and to have this functionality here soon.

archive/issue_comments_287868.json:

{
    "body": "Changed branch from **[u/cheuberg/polynomials/power](https://github.com/sagemath/sagetrac-mirror/tree/u/cheuberg/polynomials/power)** to **[u/behackl/polynomials/power](https://github.com/sagemath/sagetrac-mirror/tree/u/behackl/polynomials/power)**",
    "created_at": "2016-03-08T13:26:38Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20086#issuecomment-287868",
    "user": "https://github.com/behackl"
}

Changed branch from u/cheuberg/polynomials/power to u/behackl/polynomials/power

archive/issue_comments_287869.json:

{
    "body": "<div id=\"comment:11\" align=\"right\">comment:11</div>\n\nI've fixed the segmentation fault from `rings/integer.pyx` and implemented the case of `constant^constant`.\n\nAlso, +1 for extending this functionality on a follow-up ticket; I'd also like to have this in as soon as possible.\n\nI've reviewed your changes, please cross-review. If you are satisfied and if there are no other objections, I'd set this to `positive_review`.\n\n---\nNew commits:\n<table><tr><td><a href=\"https://github.com/sagemath/sagetrac-mirror/commit/caa02c882b08498b7796ddaa7170c8b39153821f\"><code>caa02c8</code></a></td><td><code>Merge tag '7.1.beta6' into HEAD</code></td></tr><tr><td><a href=\"https://github.com/sagemath/sagetrac-mirror/commit/3f44399e0e95e070ddf27c25fbcc4820305b26ba\"><code>3f44399</code></a></td><td><code>fix segmentation fault</code></td></tr><tr><td><a href=\"https://github.com/sagemath/sagetrac-mirror/commit/5934ed13e379e9247e68847094f9a931ead2bf0d\"><code>5934ed1</code></a></td><td><code>fix (constant poly)^(constant poly)</code></td></tr><tr><td><a href=\"https://github.com/sagemath/sagetrac-mirror/commit/e6e1c812ecb49be56dc4d9b4ed2e524011c391c9\"><code>e6e1c81</code></a></td><td><code>add doctests</code></td></tr></table>\n",
    "created_at": "2016-03-08T13:26:38Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20086#issuecomment-287869",
    "user": "https://github.com/behackl"
}

comment:11

I've fixed the segmentation fault from rings/integer.pyx and implemented the case of constant^constant.

Also, +1 for extending this functionality on a follow-up ticket; I'd also like to have this in as soon as possible.

I've reviewed your changes, please cross-review. If you are satisfied and if there are no other objections, I'd set this to positive_review.

New commits:

`caa02c8`	`Merge tag '7.1.beta6' into HEAD`
`3f44399`	`fix segmentation fault`
`5934ed1`	`fix (constant poly)^(constant poly)`
`e6e1c81`	`add doctests`

archive/issue_comments_287870.json:

{
    "body": "Changed commit from **[`918499c`](https://github.com/sagemath/sagetrac-mirror/commit/918499cf377a551a5e28ea1d6748ed50860f1afa)** to **[`e6e1c81`](https://github.com/sagemath/sagetrac-mirror/commit/e6e1c812ecb49be56dc4d9b4ed2e524011c391c9)**",
    "created_at": "2016-03-08T13:26:38Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20086#issuecomment-287870",
    "user": "https://github.com/behackl"
}

Changed commit from 918499c to e6e1c81

archive/issue_comments_287871.json:

{
    "body": "Reviewer: **Benjamin Hackl**",
    "created_at": "2016-03-08T13:27:07Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20086#issuecomment-287871",
    "user": "https://github.com/behackl"
}

Reviewer: Benjamin Hackl

archive/issue_comments_287872.json:

{
    "body": "Changed commit from **[`e6e1c81`](https://github.com/sagemath/sagetrac-mirror/commit/e6e1c812ecb49be56dc4d9b4ed2e524011c391c9)** to **[`87a22b6`](https://github.com/sagemath/sagetrac-mirror/commit/87a22b6fc4cff82491a7b0ba35983bc5dc1299df)**",
    "created_at": "2016-03-09T08:29:01Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20086#issuecomment-287872",
    "user": "https://github.com/sagetrac-git"
}

Changed commit from e6e1c81 to 87a22b6

archive/issue_comments_287873.json:

{
    "body": "<div id=\"comment:13\"></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/87a22b6fc4cff82491a7b0ba35983bc5dc1299df\"><code>87a22b6</code></a></td><td><code>fix \"referenced before assignment\"</code></td></tr></table>\n",
    "created_at": "2016-03-09T08:29:01Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20086#issuecomment-287873",
    "user": "https://github.com/sagetrac-git"
}

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

87a22b6 fix "referenced before assignment"

archive/issue_comments_287874.json:

{
    "body": "Changed branch from **[u/behackl/polynomials/power](https://github.com/sagemath/sagetrac-mirror/tree/u/behackl/polynomials/power)** to **[u/cheuberg/polynomials/power](https://github.com/sagemath/sagetrac-mirror/tree/u/cheuberg/polynomials/power)**",
    "created_at": "2016-03-09T17:00:43Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20086#issuecomment-287874",
    "user": "https://github.com/cheuberg"
}

Changed branch from u/behackl/polynomials/power to u/cheuberg/polynomials/power

archive/issue_comments_287875.json:

{
    "body": "Changed commit from **[`87a22b6`](https://github.com/sagemath/sagetrac-mirror/commit/87a22b6fc4cff82491a7b0ba35983bc5dc1299df)** to **[`14f7efa`](https://github.com/sagemath/sagetrac-mirror/commit/14f7efaaf5be81405d1954473b4dc3f47100e486)**",
    "created_at": "2016-03-09T17:03:56Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20086#issuecomment-287875",
    "user": "https://github.com/cheuberg"
}

Changed commit from 87a22b6 to 14f7efa

archive/issue_comments_287876.json:

{
    "body": "<div id=\"comment:15\" align=\"right\">comment:15</div>\n\nReplying to [@behackl](#comment%3A11):\n> I've fixed the segmentation fault from `rings/integer.pyx` and implemented the case of `constant^constant`.\n\nThat's weird, but as integer tries to convert the base to the base of the exponent and expects the polynomial ring to handle it, we have to cope with it.\n> \n> I've reviewed your changes, please cross-review. If you are satisfied and if there are no other objections, I'd set this to `positive_review`.\n\nI refactored the code: that way it seems to be more suitable for future extension and handles polynomial exponents gracefully.\n\nIn a first commit, I removed handling of the segmentation fault because I thought it should be handeled by the base ring; later on, I understood that it has to be handeled here, so re-instated fix (in some other way).\n\n---\nNew commits:\n<table><tr><td><a href=\"https://github.com/sagemath/sagetrac-mirror/commit/e95c9b9ebbfb29a4e56585d89ee839b4fa910fca\"><code>e95c9b9</code></a></td><td><code>Trac #20086: refactor method; do not fix segmentation fault 2^R in polynomial ring</code></td></tr><tr><td><a href=\"https://github.com/sagemath/sagetrac-mirror/commit/eadecbaec7b733554b996a1892d1a29421fe5153\"><code>eadecba</code></a></td><td><code>Trac #20086: fix ReSt errors (docbuild -u -k complains)</code></td></tr><tr><td><a href=\"https://github.com/sagemath/sagetrac-mirror/commit/a1398689b7e2a2694100374c4bb2373d38fe5d7c\"><code>a139868</code></a></td><td><code>Trac #20086: handle non-constant polynomial exponents</code></td></tr><tr><td><a href=\"https://github.com/sagemath/sagetrac-mirror/commit/14f7efaaf5be81405d1954473b4dc3f47100e486\"><code>14f7efa</code></a></td><td><code>Trac #20086: ReSt formatting</code></td></tr></table>\n",
    "created_at": "2016-03-09T17:03:56Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20086#issuecomment-287876",
    "user": "https://github.com/cheuberg"
}

comment:15

Replying to @behackl:

I've fixed the segmentation fault from rings/integer.pyx and implemented the case of constant^constant.

That's weird, but as integer tries to convert the base to the base of the exponent and expects the polynomial ring to handle it, we have to cope with it.

I've reviewed your changes, please cross-review. If you are satisfied and if there are no other objections, I'd set this to positive_review.

I refactored the code: that way it seems to be more suitable for future extension and handles polynomial exponents gracefully.

In a first commit, I removed handling of the segmentation fault because I thought it should be handeled by the base ring; later on, I understood that it has to be handeled here, so re-instated fix (in some other way).

New commits:

`e95c9b9`	`Trac #20086: refactor method; do not fix segmentation fault 2^R in polynomial ring`
`eadecba`	`Trac #20086: fix ReSt errors (docbuild -u -k complains)`
`a139868`	`Trac #20086: handle non-constant polynomial exponents`
`14f7efa`	`Trac #20086: ReSt formatting`

archive/issue_comments_287877.json:

{
    "body": "<div id=\"comment:16\" align=\"right\">comment:16</div>\n\nWhy doing something that complicated for dealing with strange exponents? Why not simply\n\n```\nif input is not an integer:\n  convert to an integer\ndo the exponentiation\n```\n\nWhy are you special casing degree 0 polynomial? If I understand correctly these examples won't behave similarly\n\n```\nsage: R.<x> = QQ[]\nsage: ((x+1)^2)^(1/2)\n-> TypeError\nsage: ((R(2))^2)^(1/2)\n-> 2\n```\nWhich is weird.",
    "created_at": "2016-03-09T17:36:55Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20086#issuecomment-287877",
    "user": "https://github.com/videlec"
}

comment:16

Why doing something that complicated for dealing with strange exponents? Why not simply

if input is not an integer:
  convert to an integer
do the exponentiation

Why are you special casing degree 0 polynomial? If I understand correctly these examples won't behave similarly

sage: R.<x> = QQ[]
sage: ((x+1)^2)^(1/2)
-> TypeError
sage: ((R(2))^2)^(1/2)
-> 2

Which is weird.

archive/issue_events_281140.json:

{
    "actor": "https://github.com/videlec",
    "created_at": "2016-03-09T17:36:55Z",
    "event": "unlabeled",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "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/20086#event-281140"
}

archive/issue_events_281141.json:

{
    "actor": "https://github.com/videlec",
    "created_at": "2016-03-09T17:36:55Z",
    "event": "labeled",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "label": "https://github.com/sagemath/sage/labels/needs%20info",
    "label_color": "ffff00",
    "label_name": "needs info",
    "label_text_color": "ffffff",
    "type": "issue_event",
    "url": "https://github.com/sagemath/sage/issues/20086#event-281141"
}

archive/issue_events_281142.json:

{
    "actor": "https://github.com/cheuberg",
    "created_at": "2016-03-09T17:57:44Z",
    "event": "unlabeled",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "label": "https://github.com/sagemath/sage/labels/needs%20info",
    "label_color": "ffff00",
    "label_name": "needs info",
    "label_text_color": "ffffff",
    "type": "issue_event",
    "url": "https://github.com/sagemath/sage/issues/20086#event-281142"
}

archive/issue_events_281143.json:

{
    "actor": "https://github.com/cheuberg",
    "created_at": "2016-03-09T17:57:44Z",
    "event": "labeled",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "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/20086#event-281143"
}

archive/issue_comments_287878.json:

{
    "body": "<div id=\"comment:17\" align=\"right\">comment:17</div>\n\nReplying to [@videlec](#comment%3A16):\n> Why doing something that complicated for dealing with strange exponents? Why not simply\n> \n> ```\n> if input is not an integer:\n>   convert to an integer\n> do the exponentiation\n> ```\n\nI do not understand this comment. Which input? Base or Exponent? Where should this code be?\n\n\n> \n> Why are you special casing degree 0 polynomial? If I understand correctly these examples won't behave similarly\n> \n> ```\n> sage: R.<x> = QQ[]\n> sage: ((x+1)^2)^(1/2)\n> -> TypeError\n> sage: ((R(2))^2)^(1/2)\n> -> 2\n> ```\n> Which is weird.\n\nI need a quick fix for `R(1)^(1/2)`. If somebody has time to implement `((x+1)<sup>2)</sup>(1/2)` very soon, I'd be happy. I do not have time soon. However, I want to have the code associated with a recently submitted paper in 7.1.\n\nBasically, this fix here simply branches to existing code. Computing `((x+1)<sup>2)</sup>(1/2)` needs new mathematical code (involving square free decomposition).\n\nTherefore, I propose to include this now on the basis that while this is not a perfect and definitive solution, it is better than the previous behaviour.",
    "created_at": "2016-03-09T17:57:44Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20086#issuecomment-287878",
    "user": "https://github.com/cheuberg"
}

comment:17

Replying to @videlec:

Why doing something that complicated for dealing with strange exponents? Why not simply
if input is not an integer:
  convert to an integer
do the exponentiation

I do not understand this comment. Which input? Base or Exponent? Where should this code be?

Why are you special casing degree 0 polynomial? If I understand correctly these examples won't behave similarly
sage: R.<x> = QQ[]
sage: ((x+1)^2)^(1/2)
-> TypeError
sage: ((R(2))^2)^(1/2)
-> 2
Which is weird.

I need a quick fix for R(1)^(1/2). If somebody has time to implement ((x+1)2)(1/2) very soon, I'd be happy. I do not have time soon. However, I want to have the code associated with a recently submitted paper in 7.1.

Basically, this fix here simply branches to existing code. Computing ((x+1)2)(1/2) needs new mathematical code (involving square free decomposition).

Therefore, I propose to include this now on the basis that while this is not a perfect and definitive solution, it is better than the previous behaviour.

archive/issue_comments_287879.json:

{
    "body": "<div id=\"comment:18\" align=\"right\">comment:18</div>\n\nReplying to [@cheuberg](#comment%3A17):\n> Replying to [@videlec](#comment%3A16):\n> > Why doing something that complicated for dealing with strange exponents? Why not simply\n> > \n> > ```\n> > if input is not an integer:\n> >   convert to an integer\n> > do the exponentiation\n> > ```\n> \n> \n> I do not understand this comment. Which input? Base or Exponent? Where should this code be?\n\n \nLet me be more precise then\n\n```\ndef __pow__(self, exp):\n    cdef long n\n    try:\n        n = exp\n        # old code for integer exponent\n    except TypeError:\n        n = QQ.coerce(exp)\n        # new code for rational exponent\n```\nWhat I do not understand is why are you testing if the exponent is a polynomial...\n\nThe following is currently a `TypeError`\n\n```\nsage: 1^(ZZ['x'].one())\nTraceback (most recent call last):\n...\nTypeError: 'sage.rings.polynomial.polynomial_integer_dense_flint.Polynomial_integer_dense_flint' object cannot be interpreted as an index\n```\nThe same should happen when the integer `1` is replaced by the polynomial `1`.\n\n> > Why are you special casing degree 0 polynomial? If I understand correctly these examples won't behave similarly\n> > \n> > ```\n> > sage: R.<x> = QQ[]\n> > sage: ((x+1)^2)^(1/2)\n> > -> TypeError\n> > sage: ((R(2))^2)^(1/2)\n> > -> 2\n> > ```\n> > Which is weird.\n> \n> \n> I need a quick fix for `R(1)^(1/2)`. If somebody has time to implement `((x+1)<sup>2)</sup>(1/2)` very soon, I'd be happy. I do not have time soon. However, I want to have the code associated with a recently submitted paper in 7.1.\n\nAre you sure there was a bug? In Sage the integer 1 is *not* the 1 from ZZ[x] (though they are equal through coercion). Some softwares behave differently to that respect (e.g. GAP) where there is only one 1 which is an integer and not anything else. In Sage (but not in GAP) it is already the case that operations change with respect to the parents even if the objects are equal\n\n```\nsage: Zmod(10)(3) == 3\nsage: Zmod(10)(5) == 5\nsage: log(Zmod(10)(3))\n3\nsage: log(Zmod(10)(5))\nTraceback (most recent call last):\n...\nZeroDivisionError: Inverse does not exist.\n```\n\n> Basically, this fix here simply branches to existing code.\n> Computing `((x+1)<sup>2)</sup>(1/2)` needs new mathematical code (involving square free decomposition).\n\nIndeed. Your modifications obfuscate the code for only a very trivial case and a discutable behavior of powering with polynomials.\n\n> Therefore, I propose to include this now on the basis that while this is not a perfect and definitive solution, it is better than the previous behaviour.\n\nNot sure it is better. Sage used to consider operations based on parents. Powers are of course a special type of operation and with some respect might be treated appart. But \"(polynomial)^(polynomial)\" is not well defined. And \"(polynomial)^(rational)\" is well defined in some situations (and to that respect, your code improves the current situation a little).\n\nMoreover, the current \"power promotion\" for ZZ is very bad\n\n```\nsage: 3^(1/2)\nsqrt(3)\nsage: parent(_)\nSymbolic Ring\nsage: 4^(1/2)\n2\nsage: parent(_)\nRational Field\n```\nMaking it available for constant polynomial is not that much of an improvement.",
    "created_at": "2016-03-09T18:31:55Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20086#issuecomment-287879",
    "user": "https://github.com/videlec"
}

comment:18

Replying to @cheuberg:

Replying to @videlec:
Why doing something that complicated for dealing with strange exponents? Why not simply
if input is not an integer:
  convert to an integer
do the exponentiation
I do not understand this comment. Which input? Base or Exponent? Where should this code be?

Let me be more precise then

def __pow__(self, exp):
    cdef long n
    try:
        n = exp
        # old code for integer exponent
    except TypeError:
        n = QQ.coerce(exp)
        # new code for rational exponent

What I do not understand is why are you testing if the exponent is a polynomial...

The following is currently a TypeError

sage: 1^(ZZ['x'].one())
Traceback (most recent call last):
...
TypeError: 'sage.rings.polynomial.polynomial_integer_dense_flint.Polynomial_integer_dense_flint' object cannot be interpreted as an index

The same should happen when the integer 1 is replaced by the polynomial 1.

Why are you special casing degree 0 polynomial? If I understand correctly these examples won't behave similarly
sage: R.<x> = QQ[]
sage: ((x+1)^2)^(1/2)
-> TypeError
sage: ((R(2))^2)^(1/2)
-> 2
Which is weird.
I need a quick fix for R(1)^(1/2). If somebody has time to implement ((x+1)2)(1/2) very soon, I'd be happy. I do not have time soon. However, I want to have the code associated with a recently submitted paper in 7.1.

Are you sure there was a bug? In Sage the integer 1 is not the 1 from ZZ[x] (though they are equal through coercion). Some softwares behave differently to that respect (e.g. GAP) where there is only one 1 which is an integer and not anything else. In Sage (but not in GAP) it is already the case that operations change with respect to the parents even if the objects are equal

sage: Zmod(10)(3) == 3
sage: Zmod(10)(5) == 5
sage: log(Zmod(10)(3))
3
sage: log(Zmod(10)(5))
Traceback (most recent call last):
...
ZeroDivisionError: Inverse does not exist.

Basically, this fix here simply branches to existing code. Computing ((x+1)2)(1/2) needs new mathematical code (involving square free decomposition).

Indeed. Your modifications obfuscate the code for only a very trivial case and a discutable behavior of powering with polynomials.

Therefore, I propose to include this now on the basis that while this is not a perfect and definitive solution, it is better than the previous behaviour.

Not sure it is better. Sage used to consider operations based on parents. Powers are of course a special type of operation and with some respect might be treated appart. But "(polynomial)^(polynomial)" is not well defined. And "(polynomial)^(rational)" is well defined in some situations (and to that respect, your code improves the current situation a little).

Moreover, the current "power promotion" for ZZ is very bad

sage: 3^(1/2)
sqrt(3)
sage: parent(_)
Symbolic Ring
sage: 4^(1/2)
2
sage: parent(_)
Rational Field

Making it available for constant polynomial is not that much of an improvement.

archive/issue_comments_287880.json:

{
    "body": "Description changed:\n``````diff\n--- \n+++ \n@@ -1,14 +1,11 @@\n Until now,\n \n ```\n-sage: P.<R> = QQ[]\n-sage: P(1)^(1/2)\n+sage: R.<x> = ZZ[]\n+sage: R(1)^(1/2)\n Traceback (most recent call last):\n ...\n-ArithmeticError: Cannot take T to the exponent 1/2 in Exact Term Monoid T^QQ\n-with coefficients in Univariate Polynomial Ring in R over Rational Field\n-since its coefficient 1 cannot be taken to this exponent.\n-> *previous* TypeError: rational is not an integer\n+TypeError: rational is not an integer\n ```\n because only integer exponents were allowed for polynomials.\n \n``````\n",
    "created_at": "2016-03-09T20:01:08Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20086#issuecomment-287880",
    "user": "https://github.com/cheuberg"
}

Description changed:

--- 
+++ 
@@ -1,14 +1,11 @@
 Until now,
 
 ```
-sage: P.<R> = QQ[]
-sage: P(1)^(1/2)
+sage: R.<x> = ZZ[]
+sage: R(1)^(1/2)
 Traceback (most recent call last):
 ...
-ArithmeticError: Cannot take T to the exponent 1/2 in Exact Term Monoid T^QQ
-with coefficients in Univariate Polynomial Ring in R over Rational Field
-since its coefficient 1 cannot be taken to this exponent.
-> *previous* TypeError: rational is not an integer
+TypeError: rational is not an integer
 ```
 because only integer exponents were allowed for polynomials.

archive/issue_comments_287881.json:

{
    "body": "<div id=\"comment:19\" align=\"right\">comment:19</div>\n\nIt seems that I mis-edited the description of the problem at some stage. Is now fixed.\n\nApart from that, the current code apparently leads to problems.",
    "created_at": "2016-03-09T20:01:08Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20086#issuecomment-287881",
    "user": "https://github.com/cheuberg"
}

comment:19

It seems that I mis-edited the description of the problem at some stage. Is now fixed.

Apart from that, the current code apparently leads to problems.

archive/issue_events_281144.json:

{
    "actor": "https://github.com/cheuberg",
    "created_at": "2016-03-09T20:01:08Z",
    "event": "unlabeled",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "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/20086#event-281144"
}

archive/issue_events_281145.json:

{
    "actor": "https://github.com/cheuberg",
    "created_at": "2016-03-09T20:01:08Z",
    "event": "labeled",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "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/20086#event-281145"
}

archive/issue_comments_287882.json:

{
    "body": "<div id=\"comment:20\" align=\"right\">comment:20</div>\n\nReplying to [@videlec](#comment%3A18):\n> Moreover, the current \"power promotion\" for ZZ is very bad\n> \n> ```\n> sage: 3^(1/2)\n> sqrt(3)\n> sage: parent(_)\n> Symbolic Ring\n> sage: 4^(1/2)\n> 2\n> sage: parent(_)\n> Rational Field\n> ```\n\nsee [nbruin](#comment%3A6)'s comment 6.",
    "created_at": "2016-03-09T20:03:16Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20086#issuecomment-287882",
    "user": "https://github.com/cheuberg"
}

comment:20

Replying to @videlec:

Moreover, the current "power promotion" for ZZ is very bad
sage: 3^(1/2)
sqrt(3)
sage: parent(_)
Symbolic Ring
sage: 4^(1/2)
2
sage: parent(_)
Rational Field

see nbruin's comment 6.

archive/issue_comments_287883.json:

{
    "body": "<div id=\"comment:21\" align=\"right\">comment:21</div>\n\nHi Vincent!\n\nJust to clarify the previous need for checking whether the exponent is a polynomial: in our previous approach, we were hoping for the coefficient ring to carry out the exponentiation, i.e. if we had\n\n```\nsage: P.<z> = QQ[]\nsage: P(1/4)^(1/2)\n1/2\n```\nthen we would compute this by letting `QQ` do the exponentiation. However, in the doctest over at `rings/integer.pyx` we have something like\n\n```\nsage: P.<t> = QQ[]\nsage: 2^t\n```\nWith the old implementation, this lead to an infinite loop (because coercion would put the base always back to P again...), and thus I added the check for the polynomial in the exponent.\n\nIn any case, I'm all for improving the code and first trying to deal with integers, and then with rationals afterwards. Nevertheless, I'd still just handle constant polynomials in the base on this ticket---but then, this could be extended easily.\n\nBenjamin",
    "created_at": "2016-03-09T20:08:28Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20086#issuecomment-287883",
    "user": "https://github.com/behackl"
}

comment:21

Hi Vincent!

Just to clarify the previous need for checking whether the exponent is a polynomial: in our previous approach, we were hoping for the coefficient ring to carry out the exponentiation, i.e. if we had

sage: P.<z> = QQ[]
sage: P(1/4)^(1/2)
1/2

then we would compute this by letting QQ do the exponentiation. However, in the doctest over at rings/integer.pyx we have something like

sage: P.<t> = QQ[]
sage: 2^t

With the old implementation, this lead to an infinite loop (because coercion would put the base always back to P again...), and thus I added the check for the polynomial in the exponent.

In any case, I'm all for improving the code and first trying to deal with integers, and then with rationals afterwards. Nevertheless, I'd still just handle constant polynomials in the base on this ticket---but then, this could be extended easily.

Benjamin

archive/issue_comments_287884.json:

{
    "body": "<div id=\"comment:22\"></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/892109ac4f48a4a32d35671365ecceb0d1c9e0bf\"><code>892109a</code></a></td><td><code>Trac #20086: fix bug in previous commit (and use better doctest)</code></td></tr></table>\n",
    "created_at": "2016-03-09T20:10:52Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20086#issuecomment-287884",
    "user": "https://github.com/sagetrac-git"
}

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

892109a Trac #20086: fix bug in previous commit (and use better doctest)

archive/issue_comments_287885.json:

{
    "body": "Changed commit from **[`14f7efa`](https://github.com/sagemath/sagetrac-mirror/commit/14f7efaaf5be81405d1954473b4dc3f47100e486)** to **[`892109a`](https://github.com/sagemath/sagetrac-mirror/commit/892109ac4f48a4a32d35671365ecceb0d1c9e0bf)**",
    "created_at": "2016-03-09T20:10:52Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20086#issuecomment-287885",
    "user": "https://github.com/sagetrac-git"
}

Changed commit from 14f7efa to 892109a

archive/issue_events_281146.json:

{
    "actor": "https://github.com/cheuberg",
    "created_at": "2016-03-09T20:12:01Z",
    "event": "unlabeled",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "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/20086#event-281146"
}

archive/issue_events_281147.json:

{
    "actor": "https://github.com/cheuberg",
    "created_at": "2016-03-09T20:12:01Z",
    "event": "labeled",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "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/20086#event-281147"
}

archive/issue_comments_287886.json:

{
    "body": "<div id=\"comment:23\" align=\"right\">comment:23</div>\n\nProblem found by patchbot was a stupid mistake, hopefully fixed now.",
    "created_at": "2016-03-09T20:12:01Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20086#issuecomment-287886",
    "user": "https://github.com/cheuberg"
}

comment:23

Problem found by patchbot was a stupid mistake, hopefully fixed now.

archive/issue_comments_287887.json:

{
    "body": "<div id=\"comment:24\" align=\"right\">comment:24</div>\n\nReplying to [@videlec](#comment%3A18):\n> Let me be more precise then\n> \n> ```\n> def __pow__(self, exp):\n>     cdef long n\n>     try:\n>         n = exp\n>         # old code for integer exponent\n>     except TypeError:\n>         n = QQ.coerce(exp)\n>         # new code for rational exponent\n> ```\n\nok, if you prefer it like that, we can probably do that.\n\n> > I need a quick fix for `R(1)^(1/2)`. If somebody has time to implement `((x+1)<sup>2)</sup>(1/2)` very soon, I'd be happy. I do not have time soon. However, I want to have the code associated with a recently submitted paper in 7.1.\n> \n> \n> Are you sure there was a bug?\n\nDo you think that disallowing `R(1)^(1/2)` is the desired behaviour?\n\n> In Sage the integer 1 is *not* the 1 from ZZ[x] (though they are equal through coercion). Some softwares behave differently to that respect (e.g. GAP) where there is only one 1 which is an integer and not anything else. In Sage (but not in GAP) it is already the case that operations change with respect to the parents even if the objects are equal\n> \n> ```\n> sage: Zmod(10)(3) == 3\n> sage: Zmod(10)(5) == 5\n> sage: log(Zmod(10)(3))\n> 3\n> sage: log(Zmod(10)(5))\n> Traceback (most recent call last):\n> ...\n> ZeroDivisionError: Inverse does not exist.\n> ```\n\nI have no idea how this is related to this problem, sorry.\n\n> \n> > Basically, this fix here simply branches to existing code.\n> > Computing `((x+1)<sup>2)</sup>(1/2)` needs new mathematical code (involving square free decomposition).\n> \n> \n> Indeed. Your modifications obfuscate the code for only a very trivial case and a discutable behavior of powering with polynomials.\n\nIt might seem trivial to you, but it did cost me an hour while writing a paper because basically, asymptotic rings using polynomial rings as coefficient rings could not compute square roots, and, sorry, I need that.\n\n> \n> > Therefore, I propose to include this now on the basis that while this is not a perfect and definitive solution, it is better than the previous behaviour.\n> \n> \n> Not sure it is better. Sage used to consider operations based on parents. Powers are of course a special type of operation and with some respect might be treated appart. But \"(polynomial)^(polynomial)\" is not well defined. And \"(polynomial)^(rational)\" is well defined in some situations (and to that respect, your code improves the current situation a little).\n\nThere was a discussion on sage-devel a few weeks ago. Every parent seems to have its own home-made logic about how to do coercion. Please do not try to fix that in this ticket.",
    "created_at": "2016-03-09T20:23:59Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20086#issuecomment-287887",
    "user": "https://github.com/cheuberg"
}

comment:24

Replying to @videlec:

Let me be more precise then

def __pow__(self, exp):
    cdef long n
    try:
        n = exp
        # old code for integer exponent
    except TypeError:
        n = QQ.coerce(exp)
        # new code for rational exponent

ok, if you prefer it like that, we can probably do that.

I need a quick fix for R(1)^(1/2). If somebody has time to implement ((x+1)2)(1/2) very soon, I'd be happy. I do not have time soon. However, I want to have the code associated with a recently submitted paper in 7.1.

Are you sure there was a bug?

Do you think that disallowing R(1)^(1/2) is the desired behaviour?

In Sage the integer 1 is not the 1 from ZZ[x] (though they are equal through coercion). Some softwares behave differently to that respect (e.g. GAP) where there is only one 1 which is an integer and not anything else. In Sage (but not in GAP) it is already the case that operations change with respect to the parents even if the objects are equal
sage: Zmod(10)(3) == 3
sage: Zmod(10)(5) == 5
sage: log(Zmod(10)(3))
3
sage: log(Zmod(10)(5))
Traceback (most recent call last):
...
ZeroDivisionError: Inverse does not exist.

I have no idea how this is related to this problem, sorry.

Basically, this fix here simply branches to existing code. Computing ((x+1)2)(1/2) needs new mathematical code (involving square free decomposition).

Indeed. Your modifications obfuscate the code for only a very trivial case and a discutable behavior of powering with polynomials.

It might seem trivial to you, but it did cost me an hour while writing a paper because basically, asymptotic rings using polynomial rings as coefficient rings could not compute square roots, and, sorry, I need that.

Therefore, I propose to include this now on the basis that while this is not a perfect and definitive solution, it is better than the previous behaviour.

Not sure it is better. Sage used to consider operations based on parents. Powers are of course a special type of operation and with some respect might be treated appart. But "(polynomial)^(polynomial)" is not well defined. And "(polynomial)^(rational)" is well defined in some situations (and to that respect, your code improves the current situation a little).

There was a discussion on sage-devel a few weeks ago. Every parent seems to have its own home-made logic about how to do coercion. Please do not try to fix that in this ticket.

archive/issue_comments_287888.json:

{
    "body": "<div id=\"comment:25\" align=\"right\">comment:25</div>\n\nReplying to [@cheuberg](#comment%3A24):\n> >\n> >[... snip ...]\n> >\n\n> There was a discussion on sage-devel a few weeks ago. Every parent seems to have its own home-made logic about how to do coercion. Please do not try to fix that in this ticket.\n\nPlease do not try to complicate it in this ticket either. A possible solution could be:\n- allow `polynomial^rational` as much as we can (i.e. for some constant polynomials for the sake of this ticket). For more general polynomial we can for now raise a `NotImplementedError`.\n- disallow `polynomial^polynomial` even if the power is a constant polynomial. In that case, the error should be a `TypeError`. This is the current behavior for some polynomial rings but not all\n\n```\nsage: ZZ['x'].gen() ** ZZ['x'].one()\n -> TypeError\nsage: QQ['x'].gen() ** ZZ['x'].one()\n -> TypeError\nsage: RR['x'].gen() ** ZZ['x'].one()\nx\n```\n\nWhat do you think? Would that solution fit your needs?\n\nVincent",
    "created_at": "2016-03-10T02:23:55Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20086#issuecomment-287888",
    "user": "https://github.com/videlec"
}

comment:25

Replying to @cheuberg:

[... snip ...]

There was a discussion on sage-devel a few weeks ago. Every parent seems to have its own home-made logic about how to do coercion. Please do not try to fix that in this ticket.

Please do not try to complicate it in this ticket either. A possible solution could be:

allow polynomial^rational as much as we can (i.e. for some constant polynomials for the sake of this ticket). For more general polynomial we can for now raise a NotImplementedError.
disallow polynomial^polynomial even if the power is a constant polynomial. In that case, the error should be a TypeError. This is the current behavior for some polynomial rings but not all

sage: ZZ['x'].gen() ** ZZ['x'].one()
 -> TypeError
sage: QQ['x'].gen() ** ZZ['x'].one()
 -> TypeError
sage: RR['x'].gen() ** ZZ['x'].one()
x

What do you think? Would that solution fit your needs?

Vincent

archive/issue_comments_287889.json:

{
    "body": "<div id=\"comment:26\" align=\"right\">comment:26</div>\n\nReplying to [@videlec](#comment%3A25):\n> A possible solution could be:\n> - allow `polynomial^rational` as much as we can (i.e. for some constant polynomials for the sake of this ticket). For more general polynomial we can for now raise a `NotImplementedError`.\n> - disallow `polynomial^polynomial` even if the power is a constant polynomial. In that case, the error should be a `TypeError`. This is the current behavior for some polynomial rings but not all\n> \n> ```\n> sage: ZZ['x'].gen() ** ZZ['x'].one()\n>  -> TypeError\n> sage: QQ['x'].gen() ** ZZ['x'].one()\n>  -> TypeError\n> sage: RR['x'].gen() ** ZZ['x'].one()\n> x\n> ```\n> \n> What do you think? Would that solution fit your needs?\n\nAs far as I can see without implementing it and `make ptestlong`, this fits perfectly.",
    "created_at": "2016-03-10T05:31:02Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20086#issuecomment-287889",
    "user": "https://github.com/cheuberg"
}

comment:26

Replying to @videlec:

A possible solution could be:

allow polynomial^rational as much as we can (i.e. for some constant polynomials for the sake of this ticket). For more general polynomial we can for now raise a NotImplementedError.

disallow polynomial^polynomial even if the power is a constant polynomial. In that case, the error should be a TypeError. This is the current behavior for some polynomial rings but not all
sage: ZZ['x'].gen() ** ZZ['x'].one()
 -> TypeError
sage: QQ['x'].gen() ** ZZ['x'].one()
 -> TypeError
sage: RR['x'].gen() ** ZZ['x'].one()
x
What do you think? Would that solution fit your needs?

As far as I can see without implementing it and make ptestlong, this fits perfectly.

archive/issue_comments_287890.json:

{
    "body": "<div id=\"comment:27\"></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/1782a3cc92f875fa3ac8a83395d204d2ca3d7f8c\"><code>1782a3c</code></a></td><td><code>Trac #20086: rewrite try/expect; disallow polynomial exponents</code></td></tr></table>\n",
    "created_at": "2016-03-10T06:19:47Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20086#issuecomment-287890",
    "user": "https://github.com/sagetrac-git"
}

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

1782a3c Trac #20086: rewrite try/expect; disallow polynomial exponents

archive/issue_comments_287891.json:

{
    "body": "Changed commit from **[`892109a`](https://github.com/sagemath/sagetrac-mirror/commit/892109ac4f48a4a32d35671365ecceb0d1c9e0bf)** to **[`1782a3c`](https://github.com/sagemath/sagetrac-mirror/commit/1782a3cc92f875fa3ac8a83395d204d2ca3d7f8c)**",
    "created_at": "2016-03-10T06:19:47Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20086#issuecomment-287891",
    "user": "https://github.com/sagetrac-git"
}

Changed commit from 892109a to 1782a3c

archive/issue_comments_287892.json:

{
    "body": "<div id=\"comment:28\" align=\"right\">comment:28</div>\n\nThis is now refactored as a try/expect block; polynomial exponents are now disallowed.\n\nI am not completely comfortable with the long block wrapped within `try`/`expect` because I am not sure that we catch the correct `TypeError`. What do you think about that?",
    "created_at": "2016-03-10T06:21:43Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20086#issuecomment-287892",
    "user": "https://github.com/cheuberg"
}

comment:28

This is now refactored as a try/expect block; polynomial exponents are now disallowed.

I am not completely comfortable with the long block wrapped within try/expect because I am not sure that we catch the correct TypeError. What do you think about that?

archive/issue_comments_287893.json:

{
    "body": "<div id=\"comment:29\" align=\"right\">comment:29</div>\n\nReplying to [@cheuberg](#comment%3A28):\n> This is now refactored as a try/expect block; polynomial exponents are now disallowed.\n\nThanks.\n\n> I am not completely comfortable with the long block wrapped within `try`/`expect` because I am not sure that we catch the correct `TypeError`. What do you think about that?\n\nRight. It is better to avoid long `try/except`. One possibility is\n\n```\ntry:\n    nn = pyobject_to_long(exp)\nexcept TypeError:\n    # rational code\nelse:\n    # integer code\n```",
    "created_at": "2016-03-10T13:26:58Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20086#issuecomment-287893",
    "user": "https://github.com/videlec"
}

comment:29

Replying to @cheuberg:

This is now refactored as a try/expect block; polynomial exponents are now disallowed.

Thanks.

I am not completely comfortable with the long block wrapped within try/expect because I am not sure that we catch the correct TypeError. What do you think about that?

Right. It is better to avoid long try/except. One possibility is

try:
    nn = pyobject_to_long(exp)
except TypeError:
    # rational code
else:
    # integer code

archive/issue_comments_287894.json:

{
    "body": "<div id=\"comment:30\"></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/9bda3669c14463b3061f19c959bf8a22cb467117\"><code>9bda366</code></a></td><td><code>Trac #20086: refactor as try/except/else</code></td></tr></table>\n",
    "created_at": "2016-03-10T13:45:34Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20086#issuecomment-287894",
    "user": "https://github.com/sagetrac-git"
}

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

9bda366 Trac #20086: refactor as try/except/else

archive/issue_comments_287895.json:

{
    "body": "Changed commit from **[`1782a3c`](https://github.com/sagemath/sagetrac-mirror/commit/1782a3cc92f875fa3ac8a83395d204d2ca3d7f8c)** to **[`9bda366`](https://github.com/sagemath/sagetrac-mirror/commit/9bda3669c14463b3061f19c959bf8a22cb467117)**",
    "created_at": "2016-03-10T13:45:34Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20086#issuecomment-287895",
    "user": "https://github.com/sagetrac-git"
}

Changed commit from 1782a3c to 9bda366

archive/issue_comments_287896.json:

{
    "body": "<div id=\"comment:31\" align=\"right\">comment:31</div>\n\nReplying to [@videlec](#comment%3A29):\n> Right. It is better to avoid long `try/except`. One possibility is\n> \n> ```\n> try:\n>     nn = pyobject_to_long(exp)\n> except TypeError:\n>     # rational code\n> else:\n>     # integer code\n> ```\n\ndone.",
    "created_at": "2016-03-10T13:46:34Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20086#issuecomment-287896",
    "user": "https://github.com/cheuberg"
}

comment:31

Replying to @videlec:

Right. It is better to avoid long try/except. One possibility is
try:
    nn = pyobject_to_long(exp)
except TypeError:
    # rational code
else:
    # integer code

done.

archive/issue_comments_287897.json:

{
    "body": "Changed author from **Clemens Heuberger** to **Clemens Heuberger, Vincent Delecroix**",
    "created_at": "2016-03-11T15:45:16Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20086#issuecomment-287897",
    "user": "https://github.com/videlec"
}

Changed author from Clemens Heuberger to Clemens Heuberger, Vincent Delecroix

archive/issue_comments_287898.json:

{
    "body": "Changed branch from **[u/cheuberg/polynomials/power](https://github.com/sagemath/sagetrac-mirror/tree/u/cheuberg/polynomials/power)** to **[u/vdelecroix/20086](https://github.com/sagemath/sagetrac-mirror/tree/u/vdelecroix/20086)**",
    "created_at": "2016-03-11T15:45:16Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20086#issuecomment-287898",
    "user": "https://github.com/videlec"
}

Changed branch from u/cheuberg/polynomials/power to u/vdelecroix/20086

archive/issue_comments_287899.json:

{
    "body": "<div id=\"comment:32\" align=\"right\">comment:32</div>\n\nI implemented a generic `nth_root` method and used it...\n\n---\nNew commits:\n<table><tr><td><a href=\"https://github.com/sagemath/sagetrac-mirror/commit/cba01eb6ebe741567cabc1e9e4a3dd0c94aa11b2\"><code>cba01eb</code></a></td><td><code>Trac 20086: implement (polynomial)^(rational)</code></td></tr></table>\n",
    "created_at": "2016-03-11T15:45:16Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20086#issuecomment-287899",
    "user": "https://github.com/videlec"
}

comment:32

I implemented a generic nth_root method and used it...

New commits:

cba01eb Trac 20086: implement (polynomial)^(rational)

archive/issue_events_281148.json:

{
    "actor": "https://github.com/videlec",
    "created_at": "2016-03-11T15:45:16Z",
    "event": "renamed",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "title_is": "rational powers in ZZ[X] and QQ[X]",
    "title_was": "ZZ[X], QQ[X]: allow arbitrary powers of constant polynomials",
    "type": "issue_event",
    "url": "https://github.com/sagemath/sage/issues/20086#event-281148"
}

archive/issue_comments_287900.json:

{
    "body": "Changed commit from **[`9bda366`](https://github.com/sagemath/sagetrac-mirror/commit/9bda3669c14463b3061f19c959bf8a22cb467117)** to **[`cba01eb`](https://github.com/sagemath/sagetrac-mirror/commit/cba01eb6ebe741567cabc1e9e4a3dd0c94aa11b2)**",
    "created_at": "2016-03-11T15:45:16Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20086#issuecomment-287900",
    "user": "https://github.com/videlec"
}

Changed commit from 9bda366 to cba01eb

archive/issue_comments_287901.json:

{
    "body": "<div id=\"comment:33\" align=\"right\">comment:33</div>\n\nThough, since `num` and `den` are relatively prime it should be cheaper to do `self.nth_root(den) ** num` instead of `(self ** num).nth_root(den)`... I will think about it.\n\nAnyway, it would be good to see whether it works for other polynomial rings. For example `QQbar['x']` or when the base ring is a number field or a finite field...",
    "created_at": "2016-03-11T15:49:43Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20086#issuecomment-287901",
    "user": "https://github.com/videlec"
}

comment:33

Though, since num and den are relatively prime it should be cheaper to do self.nth_root(den) ** num instead of (self ** num).nth_root(den)... I will think about it.

Anyway, it would be good to see whether it works for other polynomial rings. For example QQbar['x'] or when the base ring is a number field or a finite field...

archive/issue_comments_287902.json:

{
    "body": "<div id=\"comment:34\" align=\"right\">comment:34</div>\n\nWhen the polynomial is of degree 0 it would also be faster to actually also relies on `nth_root`:\n\n```\nsage: %timeit 27.nth_root(3)\n1000000 loops, best of 3: 388 ns per loop\nsage: %timeit 27 ** (1/3)\n100000 loops, best of 3: 7.51 \u00b5s per loop\n```\nAs `nth_root` should be guaranteed to return an element of the same ring there will also be much less coercion involved.",
    "created_at": "2016-03-11T15:53:22Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20086#issuecomment-287902",
    "user": "https://github.com/videlec"
}

comment:34

When the polynomial is of degree 0 it would also be faster to actually also relies on nth_root:

sage: %timeit 27.nth_root(3)
1000000 loops, best of 3: 388 ns per loop
sage: %timeit 27 ** (1/3)
100000 loops, best of 3: 7.51 µs per loop

As nth_root should be guaranteed to return an element of the same ring there will also be much less coercion involved.

archive/issue_comments_287903.json:

{
    "body": "Changed commit from **[`cba01eb`](https://github.com/sagemath/sagetrac-mirror/commit/cba01eb6ebe741567cabc1e9e4a3dd0c94aa11b2)** to **[`c603c2b`](https://github.com/sagemath/sagetrac-mirror/commit/c603c2b293819088246b34ab676f303871d5cb40)**",
    "created_at": "2016-03-11T15:59:49Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20086#issuecomment-287903",
    "user": "https://github.com/sagetrac-git"
}

Changed commit from cba01eb to c603c2b

archive/issue_comments_287904.json:

{
    "body": "<div id=\"comment:35\"></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/c603c2b293819088246b34ab676f303871d5cb40\"><code>c603c2b</code></a></td><td><code>Trac 20086: improvements / simplification</code></td></tr></table>\n",
    "created_at": "2016-03-11T15:59:49Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20086#issuecomment-287904",
    "user": "https://github.com/sagetrac-git"
}

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

c603c2b Trac 20086: improvements / simplification

archive/issue_comments_287905.json:

{
    "body": "Changed commit from **[`c603c2b`](https://github.com/sagemath/sagetrac-mirror/commit/c603c2b293819088246b34ab676f303871d5cb40)** to **[`42c5319`](https://github.com/sagemath/sagetrac-mirror/commit/42c53199edeb619d8b4867cea0b318d7d0c8ea0a)**",
    "created_at": "2016-03-11T17:21:57Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20086#issuecomment-287905",
    "user": "https://github.com/sagetrac-git"
}

Changed commit from c603c2b to 42c5319

archive/issue_comments_287906.json:

{
    "body": "<div id=\"comment:36\"></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/42c53199edeb619d8b4867cea0b318d7d0c8ea0a\"><code>42c5319</code></a></td><td><code>Trac 20086: fix doctest in polynomial_quotient_ring.py</code></td></tr></table>\n",
    "created_at": "2016-03-11T17:21:57Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20086#issuecomment-287906",
    "user": "https://github.com/sagetrac-git"
}

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

42c5319 Trac 20086: fix doctest in polynomial_quotient_ring.py

archive/issue_comments_287907.json:

{
    "body": "<div id=\"comment:37\" align=\"right\">comment:37</div>\n\nHi! I've reviewed this refactored implementation (thanks for implementing it!), and everything looks good to me.\n\nI've added a small reviewer commit; please cross-review and set to `positive_review` if you are satisfied.\n\n---\nNew commits:\n<table><tr><td><a href=\"https://github.com/sagemath/sagetrac-mirror/commit/4313d3f96792932aa59f8b6d763a8ba0f7948107\"><code>4313d3f</code></a></td><td><code>Merge tag '7.2.beta1' into polynomial/rational-powers</code></td></tr><tr><td><a href=\"https://github.com/sagemath/sagetrac-mirror/commit/c8350c631287dd71a9b79fb422d769a1e480cdc8\"><code>c8350c6</code></a></td><td><code>improve exception messages</code></td></tr></table>\n",
    "created_at": "2016-03-29T02:20:26Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20086#issuecomment-287907",
    "user": "https://github.com/behackl"
}

comment:37

Hi! I've reviewed this refactored implementation (thanks for implementing it!), and everything looks good to me.

I've added a small reviewer commit; please cross-review and set to positive_review if you are satisfied.

New commits:

`4313d3f`	`Merge tag '7.2.beta1' into polynomial/rational-powers`
`c8350c6`	`improve exception messages`

archive/issue_comments_287908.json:

{
    "body": "Changed commit from **[`42c5319`](https://github.com/sagemath/sagetrac-mirror/commit/42c53199edeb619d8b4867cea0b318d7d0c8ea0a)** to **[`c8350c6`](https://github.com/sagemath/sagetrac-mirror/commit/c8350c631287dd71a9b79fb422d769a1e480cdc8)**",
    "created_at": "2016-03-29T02:20:26Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20086#issuecomment-287908",
    "user": "https://github.com/behackl"
}

Changed commit from 42c5319 to c8350c6

archive/issue_comments_287909.json:

{
    "body": "Changed branch from **[u/vdelecroix/20086](https://github.com/sagemath/sagetrac-mirror/tree/u/vdelecroix/20086)** to **[u/behackl/polynomial/rational-powers](https://github.com/sagemath/sagetrac-mirror/tree/u/behackl/polynomial/rational-powers)**",
    "created_at": "2016-03-29T02:20:26Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20086#issuecomment-287909",
    "user": "https://github.com/behackl"
}

Changed branch from u/vdelecroix/20086 to u/behackl/polynomial/rational-powers

archive/issue_comments_287910.json:

{
    "body": "<div id=\"comment:38\" align=\"right\">comment:38</div>\n\nNope. This is actually working because of a bug see #20214  ;-(. What I wrote in the generic `nth_root` should always be an infinite loop (because of `u.nth_root(n)`).\n\nOne possibility:\n\n- make it work when `u.is_one()` is `True` (or more generally when `u.multiplicative_order()` is finite)\n- raise an error if `u` is not in the above case and there are some non trivial factors\n\nThe aim of the second item is that in children classes (like polynomials) you might want to do\n\n```\ndef nth_root(self, n):\n    if self.degree() <= 0:\n        # factorize unit using base ring method\n        XXX\n    else:\n        return super(XXX).nth_root(self, n)\n```",
    "created_at": "2016-03-29T02:44:05Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20086#issuecomment-287910",
    "user": "https://github.com/videlec"
}

comment:38

Nope. This is actually working because of a bug see #20214 ;-(. What I wrote in the generic nth_root should always be an infinite loop (because of u.nth_root(n)).

One possibility:

make it work when u.is_one() is True (or more generally when u.multiplicative_order() is finite)
raise an error if u is not in the above case and there are some non trivial factors

The aim of the second item is that in children classes (like polynomials) you might want to do

def nth_root(self, n):
    if self.degree() <= 0:
        # factorize unit using base ring method
        XXX
    else:
        return super(XXX).nth_root(self, n)

archive/issue_events_281149.json:

{
    "actor": "https://github.com/videlec",
    "created_at": "2016-03-29T02:44:16Z",
    "event": "unlabeled",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "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/20086#event-281149"
}

archive/issue_events_281150.json:

{
    "actor": "https://github.com/videlec",
    "created_at": "2016-03-29T02:44:16Z",
    "event": "labeled",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "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/20086#event-281150"
}

archive/issue_comments_287911.json:

{
    "body": "<div id=\"comment:40\" align=\"right\">comment:40</div>\n\nWell, wouldn't it be more natural to let the unit be an element of the base ring? (As far as I see the parent of the unit of polynomials over QQ always is from QQ, for example.)\n\nOf course, we can also add checks like if `u` is one etc., but with this ticket only polynomial rings over the rationals and the integers use this method---and both of them have implemented a `nth_root` method; this is why I still think that this would be good to go and why special treatment isn't needed. And the inconsistency mentioned in #20214 only introduces a recursion of depth 2, which would be resolved if the `unit` method would behave for integers like for rationals.\n\nIt isn't even necessary to separately implement `nth_root` for polynomials such that the case of constant polynomials is handled by the base ring: over `ZZ`, the overall coefficient is decomposed w.r.t. PFD and handled like a non-constant polynomial (which is what I would have implemented in the base ring as well). For QQ, the overall factor is in the unit and handled separately in the `nth_root` method.\n\nLetting other polynomial rings profit from this procedure should be realized in a follow-up ticket, IMHO.\n\nHowever, what should be added before this ships is a special treatment of the zero polynomial. I'll push this in a minute.",
    "created_at": "2016-03-29T07:55:46Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20086#issuecomment-287911",
    "user": "https://github.com/behackl"
}

comment:40

Well, wouldn't it be more natural to let the unit be an element of the base ring? (As far as I see the parent of the unit of polynomials over QQ always is from QQ, for example.)

Of course, we can also add checks like if u is one etc., but with this ticket only polynomial rings over the rationals and the integers use this method---and both of them have implemented a nth_root method; this is why I still think that this would be good to go and why special treatment isn't needed. And the inconsistency mentioned in #20214 only introduces a recursion of depth 2, which would be resolved if the unit method would behave for integers like for rationals.

It isn't even necessary to separately implement nth_root for polynomials such that the case of constant polynomials is handled by the base ring: over ZZ, the overall coefficient is decomposed w.r.t. PFD and handled like a non-constant polynomial (which is what I would have implemented in the base ring as well). For QQ, the overall factor is in the unit and handled separately in the nth_root method.

Letting other polynomial rings profit from this procedure should be realized in a follow-up ticket, IMHO.

However, what should be added before this ships is a special treatment of the zero polynomial. I'll push this in a minute.

archive/issue_comments_287912.json:

{
    "body": "<div id=\"comment:41\"></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/5bdbd3232fb25bda9de40d7a8189036eb80b0c19\"><code>5bdbd32</code></a></td><td><code>special treatment for zero polynomial</code></td></tr></table>\n",
    "created_at": "2016-03-29T08:11:25Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20086#issuecomment-287912",
    "user": "https://github.com/sagetrac-git"
}

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

5bdbd32 special treatment for zero polynomial

archive/issue_comments_287913.json:

{
    "body": "Changed commit from **[`c8350c6`](https://github.com/sagemath/sagetrac-mirror/commit/c8350c631287dd71a9b79fb422d769a1e480cdc8)** to **[`5bdbd32`](https://github.com/sagemath/sagetrac-mirror/commit/5bdbd3232fb25bda9de40d7a8189036eb80b0c19)**",
    "created_at": "2016-03-29T08:11:25Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20086#issuecomment-287913",
    "user": "https://github.com/sagetrac-git"
}

Changed commit from c8350c6 to 5bdbd32

archive/issue_comments_287914.json:

{
    "body": "<div id=\"comment:42\" align=\"right\">comment:42</div>\n\n... any new thoughts on this?",
    "created_at": "2016-04-18T14:57:15Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20086#issuecomment-287914",
    "user": "https://github.com/behackl"
}

comment:42

... any new thoughts on this?

archive/issue_comments_287915.json:

{
    "body": "<div id=\"comment:43\" align=\"right\">comment:43</div>\n\nReplying to [@behackl](#comment%3A42):\n> ... any new thoughts on this?\n\nI would very much like to have this in `7.2`, because I/we need it for a paper we have written. Thus I'd like to discuss this once again:\n\nIIRC the possibly infinite recursion could be avoided altogether if we would convert the unit `f.unit()` to the corresponding base ring. This wouldn't require any additional fix for the units, and the code affects only `ZZ[]` and `QQ[]` anyway, so I'm rather sure that it works.\n\nAny objections to this approach?",
    "created_at": "2016-04-30T16:17:37Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20086#issuecomment-287915",
    "user": "https://github.com/behackl"
}

comment:43

Replying to @behackl:

... any new thoughts on this?

I would very much like to have this in 7.2, because I/we need it for a paper we have written. Thus I'd like to discuss this once again:

IIRC the possibly infinite recursion could be avoided altogether if we would convert the unit f.unit() to the corresponding base ring. This wouldn't require any additional fix for the units, and the code affects only ZZ[] and QQ[] anyway, so I'm rather sure that it works.

Any objections to this approach?

archive/issue_events_281151.json:

{
    "actor": "https://github.com/behackl",
    "created_at": "2016-04-30T16:17:37Z",
    "event": "unlabeled",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "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/20086#event-281151"
}

archive/issue_events_281152.json:

{
    "actor": "https://github.com/behackl",
    "created_at": "2016-04-30T16:17:37Z",
    "event": "labeled",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "label": "https://github.com/sagemath/sage/labels/needs%20info",
    "label_color": "ffff00",
    "label_name": "needs info",
    "label_text_color": "ffffff",
    "type": "issue_event",
    "url": "https://github.com/sagemath/sage/issues/20086#event-281152"
}

archive/issue_comments_287916.json:

{
    "body": "<div id=\"comment:44\" align=\"right\">comment:44</div>\n\nIt is fine for me if you modify the generic `nth_root` to\n\n```\nu = f.unit()\nif u.is_one():\n    # do nothing\nelif self.parent() != self.base_ring():\n    # try to factorize the unit in the base ring\nelse:\n    # raise a NotImplementedError\n```\n\nEDIT: small modif in the code `self != self.base_ring()` -> `self.parent() != self.base_ring()`",
    "created_at": "2016-04-30T18:33:40Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20086#issuecomment-287916",
    "user": "https://github.com/videlec"
}

comment:44

It is fine for me if you modify the generic nth_root to

u = f.unit()
if u.is_one():
    # do nothing
elif self.parent() != self.base_ring():
    # try to factorize the unit in the base ring
else:
    # raise a NotImplementedError

EDIT: small modif in the code self != self.base_ring() -> self.parent() != self.base_ring()

archive/issue_comments_287917.json:

{
    "body": "Changed commit from **[`5bdbd32`](https://github.com/sagemath/sagetrac-mirror/commit/5bdbd3232fb25bda9de40d7a8189036eb80b0c19)** to **[`6bf385b`](https://github.com/sagemath/sagetrac-mirror/commit/6bf385b18875572eeb4ab748eb8f25124bce6acf)**",
    "created_at": "2016-04-30T23:47:58Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20086#issuecomment-287917",
    "user": "https://github.com/sagetrac-git"
}

Changed commit from 5bdbd32 to 6bf385b

archive/issue_comments_287918.json:

{
    "body": "<div id=\"comment:45\"></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/88dced3d4cabe15d0260e6bfdeecdcc73e4df012\"><code>88dced3</code></a></td><td><code>Merge tag '7.2.beta6' into polynomial/rational-powers</code></td></tr><tr><td><a href=\"https://github.com/sagemath/sagetrac-mirror/commit/6bf385b18875572eeb4ab748eb8f25124bce6acf\"><code>6bf385b</code></a></td><td><code>handle unit more carefully</code></td></tr></table>\n",
    "created_at": "2016-04-30T23:47:58Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20086#issuecomment-287918",
    "user": "https://github.com/sagetrac-git"
}

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

`88dced3`	`Merge tag '7.2.beta6' into polynomial/rational-powers`
`6bf385b`	`handle unit more carefully`

archive/issue_comments_287919.json:

{
    "body": "<div id=\"comment:46\" align=\"right\">comment:46</div>\n\nReplying to [@videlec](#comment%3A44):\n> It is fine for me if you modify the generic `nth_root` to\n> \n> ```\n> u = f.unit()\n> if u.is_one():\n>     # do nothing\n> elif self.parent() != self.base_ring():\n>     # try to factorize the unit in the base ring\n> else:\n>     # raise a NotImplementedError\n> ```\n> \n> EDIT: small modif in the code `self != self.base_ring()` -> `self.parent() != self.base_ring()`\n\nThanks for the suggestion! I've pushed some changes such that the unit is handled with more care. Apart from that, I've reviewed the documentation changes and doctests already in the past, this is still fine for me. Please cross-review. :-)",
    "created_at": "2016-04-30T23:53:06Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20086#issuecomment-287919",
    "user": "https://github.com/behackl"
}

comment:46

Replying to @videlec:

It is fine for me if you modify the generic nth_root to
u = f.unit()
if u.is_one():
    # do nothing
elif self.parent() != self.base_ring():
    # try to factorize the unit in the base ring
else:
    # raise a NotImplementedError
EDIT: small modif in the code self != self.base_ring() -> self.parent() != self.base_ring()

Thanks for the suggestion! I've pushed some changes such that the unit is handled with more care. Apart from that, I've reviewed the documentation changes and doctests already in the past, this is still fine for me. Please cross-review. :-)

archive/issue_events_281153.json:

{
    "actor": "https://github.com/behackl",
    "created_at": "2016-04-30T23:53:06Z",
    "event": "unlabeled",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "label": "https://github.com/sagemath/sage/labels/needs%20info",
    "label_color": "ffff00",
    "label_name": "needs info",
    "label_text_color": "ffffff",
    "type": "issue_event",
    "url": "https://github.com/sagemath/sage/issues/20086#event-281153"
}

archive/issue_events_281154.json:

{
    "actor": "https://github.com/behackl",
    "created_at": "2016-04-30T23:53:06Z",
    "event": "labeled",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "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/20086#event-281154"
}

archive/issue_comments_287920.json:

{
    "body": "<div id=\"comment:47\" align=\"right\">comment:47</div>\n\nWith\n\n```\nif u.parent() != u.base_ring():\n    u = u.base_ring(u)\n\ntry:\n    ans = u.nth_root(n)\nexcept AttributeError:\n    raise NotImplementedError(...)\n```\nYou have a risk of infinite recursion. You should use `nth_root` of `u` only if `u.parent()` is not `self.parent()` anymore.",
    "created_at": "2016-05-01T03:54:11Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20086#issuecomment-287920",
    "user": "https://github.com/videlec"
}

comment:47

With

if u.parent() != u.base_ring():
    u = u.base_ring(u)

try:
    ans = u.nth_root(n)
except AttributeError:
    raise NotImplementedError(...)

You have a risk of infinite recursion. You should use nth_root of u only if u.parent() is not self.parent() anymore.

archive/issue_comments_287921.json:

{
    "body": "<div id=\"comment:48\" align=\"right\">comment:48</div>\n\nMoreover, it is not always possible to do `u = u.base_ring(u)` (even for units).",
    "created_at": "2016-05-01T03:55:04Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20086#issuecomment-287921",
    "user": "https://github.com/videlec"
}

comment:48

Moreover, it is not always possible to do u = u.base_ring(u) (even for units).

archive/issue_comments_287922.json:

{
    "body": "Changed branch from **[u/behackl/polynomial/rational-powers](https://github.com/sagemath/sagetrac-mirror/tree/u/behackl/polynomial/rational-powers)** to **[u/public/20086](https://github.com/sagemath/sagetrac-mirror/tree/u/public/20086)**",
    "created_at": "2016-05-01T04:27:33Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20086#issuecomment-287922",
    "user": "https://github.com/videlec"
}

Changed branch from u/behackl/polynomial/rational-powers to u/public/20086

archive/issue_comments_287923.json:

{
    "body": "Changed commit from **[`6bf385b`](https://github.com/sagemath/sagetrac-mirror/commit/6bf385b18875572eeb4ab748eb8f25124bce6acf)** to none",
    "created_at": "2016-05-01T04:27:33Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20086#issuecomment-287923",
    "user": "https://github.com/videlec"
}

Changed commit from 6bf385b to none

archive/issue_events_281155.json:

{
    "actor": "https://github.com/videlec",
    "created_at": "2016-05-01T04:27:33Z",
    "event": "demilestoned",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "milestone_number": null,
    "milestone_title": "sage-7.1",
    "type": "issue_event",
    "url": "https://github.com/sagemath/sage/issues/20086#event-281155"
}

archive/issue_events_281156.json:

{
    "actor": "https://github.com/videlec",
    "created_at": "2016-05-01T04:27:33Z",
    "event": "milestoned",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "milestone_number": null,
    "milestone_title": "sage-7.2",
    "type": "issue_event",
    "url": "https://github.com/sagemath/sage/issues/20086#event-281156"
}

archive/issue_comments_287924.json:

{
    "body": "Commit: **[`654e207`](https://github.com/sagemath/sagetrac-mirror/commit/654e207cd5b003c4bae5edbde67c5b49330646f8)**",
    "created_at": "2016-05-01T04:28:53Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20086#issuecomment-287924",
    "user": "https://github.com/videlec"
}

Commit: 654e207

archive/issue_comments_287925.json:

{
    "body": "Changed branch from **[u/public/20086](https://github.com/sagemath/sagetrac-mirror/tree/u/public/20086)** to **[public/20086](https://github.com/sagemath/sagetrac-mirror/tree/public/20086)**",
    "created_at": "2016-05-01T04:28:53Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20086#issuecomment-287925",
    "user": "https://github.com/videlec"
}

Changed branch from u/public/20086 to public/20086

archive/issue_comments_287926.json:

{
    "body": "<div id=\"comment:50\" align=\"right\">comment:50</div>\n\nI added a commit to fix the issues I mentioned. As a reviewer, I think that the code lacks example. It currently only tests `ZZ[x]` but is intended to work for any unique factorization domain...\n\n---\nLast 10 new commits:\n<table><tr><td><a href=\"https://github.com/sagemath/sagetrac-mirror/commit/9bda3669c14463b3061f19c959bf8a22cb467117\"><code>9bda366</code></a></td><td><code>Trac #20086: refactor as try/except/else</code></td></tr><tr><td><a href=\"https://github.com/sagemath/sagetrac-mirror/commit/cba01eb6ebe741567cabc1e9e4a3dd0c94aa11b2\"><code>cba01eb</code></a></td><td><code>Trac 20086: implement (polynomial)^(rational)</code></td></tr><tr><td><a href=\"https://github.com/sagemath/sagetrac-mirror/commit/c603c2b293819088246b34ab676f303871d5cb40\"><code>c603c2b</code></a></td><td><code>Trac 20086: improvements / simplification</code></td></tr><tr><td><a href=\"https://github.com/sagemath/sagetrac-mirror/commit/42c53199edeb619d8b4867cea0b318d7d0c8ea0a\"><code>42c5319</code></a></td><td><code>Trac 20086: fix doctest in polynomial_quotient_ring.py</code></td></tr><tr><td><a href=\"https://github.com/sagemath/sagetrac-mirror/commit/4313d3f96792932aa59f8b6d763a8ba0f7948107\"><code>4313d3f</code></a></td><td><code>Merge tag '7.2.beta1' into polynomial/rational-powers</code></td></tr><tr><td><a href=\"https://github.com/sagemath/sagetrac-mirror/commit/c8350c631287dd71a9b79fb422d769a1e480cdc8\"><code>c8350c6</code></a></td><td><code>improve exception messages</code></td></tr><tr><td><a href=\"https://github.com/sagemath/sagetrac-mirror/commit/5bdbd3232fb25bda9de40d7a8189036eb80b0c19\"><code>5bdbd32</code></a></td><td><code>special treatment for zero polynomial</code></td></tr><tr><td><a href=\"https://github.com/sagemath/sagetrac-mirror/commit/88dced3d4cabe15d0260e6bfdeecdcc73e4df012\"><code>88dced3</code></a></td><td><code>Merge tag '7.2.beta6' into polynomial/rational-powers</code></td></tr><tr><td><a href=\"https://github.com/sagemath/sagetrac-mirror/commit/6bf385b18875572eeb4ab748eb8f25124bce6acf\"><code>6bf385b</code></a></td><td><code>handle unit more carefully</code></td></tr><tr><td><a href=\"https://github.com/sagemath/sagetrac-mirror/commit/654e207cd5b003c4bae5edbde67c5b49330646f8\"><code>654e207</code></a></td><td><code>Trac 20086: fix some issues with nth_root</code></td></tr></table>\n",
    "created_at": "2016-05-01T04:28:53Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20086#issuecomment-287926",
    "user": "https://github.com/videlec"
}

comment:50

I added a commit to fix the issues I mentioned. As a reviewer, I think that the code lacks example. It currently only tests ZZ[x] but is intended to work for any unique factorization domain...

Last 10 new commits:

`9bda366`	`Trac #20086: refactor as try/except/else`
`cba01eb`	`Trac 20086: implement (polynomial)^(rational)`
`c603c2b`	`Trac 20086: improvements / simplification`
`42c5319`	`Trac 20086: fix doctest in polynomial_quotient_ring.py`
`4313d3f`	`Merge tag '7.2.beta1' into polynomial/rational-powers`
`c8350c6`	`improve exception messages`
`5bdbd32`	`special treatment for zero polynomial`
`88dced3`	`Merge tag '7.2.beta6' into polynomial/rational-powers`
`6bf385b`	`handle unit more carefully`
`654e207`	`Trac 20086: fix some issues with nth_root`

archive/issue_comments_287927.json:

{
    "body": "Changed commit from **[`654e207`](https://github.com/sagemath/sagetrac-mirror/commit/654e207cd5b003c4bae5edbde67c5b49330646f8)** to **[`49fe88e`](https://github.com/sagemath/sagetrac-mirror/commit/49fe88e4a5f7058abd072f36cba075b698586841)**",
    "created_at": "2016-05-01T10:17:56Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20086#issuecomment-287927",
    "user": "https://github.com/sagetrac-git"
}

Changed commit from 654e207 to 49fe88e

archive/issue_comments_287928.json:

{
    "body": "<div id=\"comment:51\"></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/fb17388ac7d02f905175a80807bf6f13cc71faf2\"><code>fb17388</code></a></td><td><code>remove double if, minor adaptions</code></td></tr><tr><td><a href=\"https://github.com/sagemath/sagetrac-mirror/commit/49fe88e4a5f7058abd072f36cba075b698586841\"><code>49fe88e</code></a></td><td><code>add more doctests</code></td></tr></table>\n",
    "created_at": "2016-05-01T10:17:56Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20086#issuecomment-287928",
    "user": "https://github.com/sagetrac-git"
}

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

`fb17388`	`remove double if, minor adaptions`
`49fe88e`	`add more doctests`

archive/issue_comments_287929.json:

{
    "body": "<div id=\"comment:52\" align=\"right\">comment:52</div>\n\nReplying to [@videlec](#comment%3A50):\n> I added a commit to fix the issues I mentioned. As a reviewer, I think that the code lacks example. It currently only tests `ZZ[x]` but is intended to work for any unique factorization domain...\n\nI wasn't too happy with the repeated `if` and `raise` statements, so I tried to clean that up a bit. Also, I've added doctests for `QQ[x]`, multivariate polynomial rings, and the number field in `x^2 - 2`. Does anything more exotic come to your mind that you would like to have tested as well?",
    "created_at": "2016-05-01T10:22:20Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20086#issuecomment-287929",
    "user": "https://github.com/behackl"
}

comment:52

Replying to @videlec:

I added a commit to fix the issues I mentioned. As a reviewer, I think that the code lacks example. It currently only tests ZZ[x] but is intended to work for any unique factorization domain...

I wasn't too happy with the repeated if and raise statements, so I tried to clean that up a bit. Also, I've added doctests for QQ[x], multivariate polynomial rings, and the number field in x^2 - 2. Does anything more exotic come to your mind that you would like to have tested as well?

archive/issue_comments_287930.json:

{
    "body": "<div id=\"comment:53\" align=\"right\">comment:53</div>\n\nWould be nice to have a non polynomial examples. But currently, order in number fields does not know whether they are principal ideal domain or unique factorization domain\n\n```\nsage: R = ZZ[I]\nsage: R in PrincipalIdealDomains()\nFalse\nsage: R in UniqueFactorizationDomains()\nFalse\n```\n\nIn `__pow__` for integer polynomials, in the case the exponent is an integer you should only use `nn` and not `exp` (i.e. you should replace `if exp == 0` and `if exp < 0` by `if nn == 0` and `nn < 0`).",
    "created_at": "2016-05-01T13:09:26Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20086#issuecomment-287930",
    "user": "https://github.com/videlec"
}

comment:53

Would be nice to have a non polynomial examples. But currently, order in number fields does not know whether they are principal ideal domain or unique factorization domain

sage: R = ZZ[I]
sage: R in PrincipalIdealDomains()
False
sage: R in UniqueFactorizationDomains()
False

In __pow__ for integer polynomials, in the case the exponent is an integer you should only use nn and not exp (i.e. you should replace if exp == 0 and if exp < 0 by if nn == 0 and nn < 0).

archive/issue_comments_287931.json:

{
    "body": "Changed commit from **[`49fe88e`](https://github.com/sagemath/sagetrac-mirror/commit/49fe88e4a5f7058abd072f36cba075b698586841)** to **[`6f91df9`](https://github.com/sagemath/sagetrac-mirror/commit/6f91df97a81fa094c04583314ad38cd5fd199cdb)**",
    "created_at": "2016-05-01T13:29:25Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20086#issuecomment-287931",
    "user": "https://github.com/sagetrac-git"
}

Changed commit from 49fe88e to 6f91df9

archive/issue_comments_287932.json:

{
    "body": "<div id=\"comment:54\"></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/6f91df97a81fa094c04583314ad38cd5fd199cdb\"><code>6f91df9</code></a></td><td><code>exp --> nn</code></td></tr></table>\n",
    "created_at": "2016-05-01T13:29:25Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20086#issuecomment-287932",
    "user": "https://github.com/sagetrac-git"
}

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

6f91df9 exp --> nn

archive/issue_comments_287933.json:

{
    "body": "<div id=\"comment:55\" align=\"right\">comment:55</div>\n\nReplying to [@videlec](#comment%3A53):\n> Would be nice to have a non polynomial examples. But currently, order in number fields does not know whether they are principal ideal domain or unique factorization domain\n> \n> ```\n> sage: R = ZZ[I]\n> sage: R in PrincipalIdealDomains()\n> False\n> sage: R in UniqueFactorizationDomains()\n> False\n> ```\n> \n\nYes, the gaussian integers were also my first idea for such an example, but getting this to work should rather be a follow-up ticket.\n\n> In `__pow__` for integer polynomials, in the case the exponent is an integer you should only use `nn` and not `exp` (i.e. you should replace `if exp == 0` and `if exp < 0` by `if nn == 0` and `nn < 0`).\n\nMakes sense. Done.",
    "created_at": "2016-05-01T13:37:53Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20086#issuecomment-287933",
    "user": "https://github.com/behackl"
}

comment:55

Replying to @videlec:

Would be nice to have a non polynomial examples. But currently, order in number fields does not know whether they are principal ideal domain or unique factorization domain
sage: R = ZZ[I]
sage: R in PrincipalIdealDomains()
False
sage: R in UniqueFactorizationDomains()
False

Yes, the gaussian integers were also my first idea for such an example, but getting this to work should rather be a follow-up ticket.

In __pow__ for integer polynomials, in the case the exponent is an integer you should only use nn and not exp (i.e. you should replace if exp == 0 and if exp < 0 by if nn == 0 and nn < 0).

Makes sense. Done.

archive/issue_events_281157.json:

{
    "actor": "https://github.com/videlec",
    "created_at": "2016-05-01T15:55:53Z",
    "event": "unlabeled",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "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/20086#event-281157"
}

archive/issue_events_281158.json:

{
    "actor": "https://github.com/videlec",
    "created_at": "2016-05-01T15:55:53Z",
    "event": "labeled",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "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/20086#event-281158"
}

archive/issue_comments_287934.json:

{
    "body": "Changed reviewer from **Benjamin Hackl** to **Benjamin Hackl, Vincent Delecroix**",
    "created_at": "2016-05-01T15:55:53Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20086#issuecomment-287934",
    "user": "https://github.com/videlec"
}

Changed reviewer from Benjamin Hackl to Benjamin Hackl, Vincent Delecroix

archive/issue_comments_287935.json:

{
    "body": "Changed author from **Clemens Heuberger, Vincent Delecroix** to **Clemens Heuberger, Vincent Delecroix, Benjamin Hackl**",
    "created_at": "2016-05-01T15:55:53Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20086#issuecomment-287935",
    "user": "https://github.com/videlec"
}

Changed author from Clemens Heuberger, Vincent Delecroix to Clemens Heuberger, Vincent Delecroix, Benjamin Hackl

archive/issue_comments_287936.json:

{
    "body": "<div id=\"comment:56\" align=\"right\">comment:56</div>\n\nEnough to go!",
    "created_at": "2016-05-01T15:55:53Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20086#issuecomment-287936",
    "user": "https://github.com/videlec"
}

comment:56

Enough to go!

archive/issue_comments_287937.json:

{
    "body": "<div id=\"comment:57\" align=\"right\">comment:57</div>\n\nReplying to [@videlec](#comment%3A56):\n> Enough to go!\n\nThanks for this final sprint! Now I can sleep a bit better... ;-)",
    "created_at": "2016-05-01T16:02:54Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20086#issuecomment-287937",
    "user": "https://github.com/behackl"
}

comment:57

Replying to @videlec:

Enough to go!

Thanks for this final sprint! Now I can sleep a bit better... ;-)

archive/issue_events_281159.json:

{
    "actor": "https://github.com/nbruin",
    "created_at": "2016-05-01T16:18:18Z",
    "event": "unlabeled",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "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/20086#event-281159"
}

archive/issue_events_281160.json:

{
    "actor": "https://github.com/nbruin",
    "created_at": "2016-05-01T16:18:18Z",
    "event": "labeled",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "label": "https://github.com/sagemath/sage/labels/needs%20info",
    "label_color": "ffff00",
    "label_name": "needs info",
    "label_text_color": "ffffff",
    "type": "issue_event",
    "url": "https://github.com/sagemath/sage/issues/20086#event-281160"
}

archive/issue_comments_287938.json:

{
    "body": "<div id=\"comment:58\" align=\"right\">comment:58</div>\n\nTaking an n-th root of an element in a ring R by computing a factorization is an insane way to go about it. A much better generic strategy is to hope that the univariate polynomial ring over R has a root finding algorithm and see if the polynomial `x^n-a` has a root. You will see that:\n- it actually has a decent performance over QQ (although there the algorithm should really be special-cased)\n- it will work over most fields, including the ones that are not constructed as fraction fields of rings with a factorization algorithm.\n- you don't have to mess around with the unit part that a factorization algorithm probably won't recognize.\n\nIllustration:\n\n```\nsage: a=next_prime(10^10)*next_prime(10^11)\nsage: b=a^2\nsage: %timeit (R.0^2-b).roots()\n1000 loops, best of 3: 428 \u00b5s per loop\nsage: %timeit b.factor()\n100 loops, best of 3: 3.68 ms per loop\n```\n\n```\nsage: k.<r>=NumberField(x^2+5)\nsage: b=k(7^2)\nsage: (k['x'].0^2-b).roots()\n[(7, 1), (-7, 1)]\nsage: factor(b) #doesn't work of course\n```",
    "created_at": "2016-05-01T16:18:18Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20086#issuecomment-287938",
    "user": "https://github.com/nbruin"
}

comment:58

Taking an n-th root of an element in a ring R by computing a factorization is an insane way to go about it. A much better generic strategy is to hope that the univariate polynomial ring over R has a root finding algorithm and see if the polynomial x^n-a has a root. You will see that:

it actually has a decent performance over QQ (although there the algorithm should really be special-cased)
it will work over most fields, including the ones that are not constructed as fraction fields of rings with a factorization algorithm.
you don't have to mess around with the unit part that a factorization algorithm probably won't recognize.

Illustration:

sage: a=next_prime(10^10)*next_prime(10^11)
sage: b=a^2
sage: %timeit (R.0^2-b).roots()
1000 loops, best of 3: 428 µs per loop
sage: %timeit b.factor()
100 loops, best of 3: 3.68 ms per loop

sage: k.<r>=NumberField(x^2+5)
sage: b=k(7^2)
sage: (k['x'].0^2-b).roots()
[(7, 1), (-7, 1)]
sage: factor(b) #doesn't work of course

archive/issue_comments_287939.json:

{
    "body": "<div id=\"comment:59\" align=\"right\">comment:59</div>\n\nReplying to [@nbruin](#comment%3A58):\n> Taking an n-th root of an element in a ring R by computing a factorization is an insane way to go about it. A much better generic strategy is to hope that the univariate polynomial ring over R has a root finding algorithm and see if the polynomial `x^n-a` has a root. You will see that:\n> - it actually has a decent performance over QQ (although there the algorithm should really be special-cases)\n> - it will work over most fields, including the ones that are not constructed as fraction fields of rings with a factorization algorithm.\n> - you don't have to mess around with the unit part that a factorization algorithm probably won't recognize.\n\nWhile I like the general idea of this approach, I'm for discussing it on a follow-up ticket; see it as a \"performance enhancement\" of this implementation. Also, I'm not sure how exactly the root-finding algorithm for these univariate polynomial rings can be properly motivated, for me neither `roots()` nor `any_root()` did the job:\n\n```\nsage: R.<x> = QQ[]\nsage: P.<X> = R[]\nsage: a = X^2 - (x^2 + 2*x + 1)\nsage: a.any_root(R)\nTraceback (most recent call last):\n...\nTypeError: Unable to coerce Principal ideal (1) of Univariate Polynomial Ring in x over Rational Field (<class 'sage.rings.polynomial.ideal.Ideal_1poly_field'>) to Rational\n```\n\nDo you strongly object against setting this back to `positive_review`?",
    "created_at": "2016-05-01T16:55:16Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20086#issuecomment-287939",
    "user": "https://github.com/behackl"
}

comment:59

Replying to @nbruin:

Taking an n-th root of an element in a ring R by computing a factorization is an insane way to go about it. A much better generic strategy is to hope that the univariate polynomial ring over R has a root finding algorithm and see if the polynomial x^n-a has a root. You will see that:

it actually has a decent performance over QQ (although there the algorithm should really be special-cases)

it will work over most fields, including the ones that are not constructed as fraction fields of rings with a factorization algorithm.

you don't have to mess around with the unit part that a factorization algorithm probably won't recognize.

While I like the general idea of this approach, I'm for discussing it on a follow-up ticket; see it as a "performance enhancement" of this implementation. Also, I'm not sure how exactly the root-finding algorithm for these univariate polynomial rings can be properly motivated, for me neither roots() nor any_root() did the job:

sage: R.<x> = QQ[]
sage: P.<X> = R[]
sage: a = X^2 - (x^2 + 2*x + 1)
sage: a.any_root(R)
Traceback (most recent call last):
...
TypeError: Unable to coerce Principal ideal (1) of Univariate Polynomial Ring in x over Rational Field (<class 'sage.rings.polynomial.ideal.Ideal_1poly_field'>) to Rational

Do you strongly object against setting this back to positive_review?

archive/issue_comments_287940.json:

{
    "body": "<div id=\"comment:60\" align=\"right\">comment:60</div>\n\nReplying to [@behackl](#comment%3A59):\n> Do you strongly object against setting this back to `positive_review`?\n\nYes, because it is parking code in the wrong spot.\n\nPerhaps park your code on `sage.rings.polynomial.polynomial_element.Polynomial`? Taking an n-th root of a polynomial via factorization isn't quite as bad as in general.\n\nSince `NumberFieldElement` etc. already provides an `nth_root` method, it may be sufficient to just provide the method on Polynomial.",
    "created_at": "2016-05-02T04:42:54Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20086#issuecomment-287940",
    "user": "https://github.com/nbruin"
}

comment:60

Replying to @behackl:

Do you strongly object against setting this back to positive_review?

Yes, because it is parking code in the wrong spot.

Perhaps park your code on sage.rings.polynomial.polynomial_element.Polynomial? Taking an n-th root of a polynomial via factorization isn't quite as bad as in general.

Since NumberFieldElement etc. already provides an nth_root method, it may be sufficient to just provide the method on Polynomial.

archive/issue_comments_287941.json:

{
    "body": "<div id=\"comment:61\" align=\"right\">comment:61</div>\n\nReplying to [@nbruin](#comment%3A60):\n> Replying to [@behackl](#comment%3A59):\n> > Do you strongly object against setting this back to `positive_review`?\n> \n> Yes, because it is parking code in the wrong spot.\n> \n\nI was hoping for a comment from Vincent, as he started with the generic solution on this branch.\n\nIn any case: I do not quite understand why this would be in the wrong place. Unique factorization domains should provide a `factor`-method, and providing a generic `nth_root` method for them doesn't strike me as bad.\n\nIt might be true that this ticket only changes the behavior of polynomial rings, but I don't see a downside of providing a generic solution. But maybe I'm missing something? :-)\n\n> Perhaps park your code on `sage.rings.polynomial.polynomial_element.Polynomial`? Taking an n-th root of a polynomial via factorization isn't quite as bad as in general.\n> \n> Since `NumberFieldElement` etc. already provides an `nth_root` method, it may be sufficient to just provide the method on Polynomial.\n\nYes, that is true---however, I think that the code provided on this ticket is generic enough to work for all unique factorization domains. Or doesn't it? Even if it might be not performant, I think that a generic approach is certainly allowed to be slow---this problem can be handled by special case implementations.\n\nI just don't see a benefit from moving the code back to polynomials only. Could you elaborate more why this generic solution should be degraded to a special case?",
    "created_at": "2016-05-02T18:51:10Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20086#issuecomment-287941",
    "user": "https://github.com/behackl"
}

comment:61

Replying to @nbruin:

Replying to @behackl:

Do you strongly object against setting this back to positive_review?

Yes, because it is parking code in the wrong spot.

I was hoping for a comment from Vincent, as he started with the generic solution on this branch.

In any case: I do not quite understand why this would be in the wrong place. Unique factorization domains should provide a factor-method, and providing a generic nth_root method for them doesn't strike me as bad.

It might be true that this ticket only changes the behavior of polynomial rings, but I don't see a downside of providing a generic solution. But maybe I'm missing something? :-)

Perhaps park your code on sage.rings.polynomial.polynomial_element.Polynomial? Taking an n-th root of a polynomial via factorization isn't quite as bad as in general.

Since NumberFieldElement etc. already provides an nth_root method, it may be sufficient to just provide the method on Polynomial.

Yes, that is true---however, I think that the code provided on this ticket is generic enough to work for all unique factorization domains. Or doesn't it? Even if it might be not performant, I think that a generic approach is certainly allowed to be slow---this problem can be handled by special case implementations.

I just don't see a benefit from moving the code back to polynomials only. Could you elaborate more why this generic solution should be degraded to a special case?

archive/issue_comments_287942.json:

{
    "body": "<div id=\"comment:62\" align=\"right\">comment:62</div>\n\nThe code provided is far to be working on any UFD as you have to factor the unit! But we know for sure that it will work for polynomial rings whose base ring provides a `nth_root` method. It would make sense to move the code to generic polynomial. As there is not a lot of examples of UFD in Sage it is hard to say that this code is useful for general UFD.",
    "created_at": "2016-05-02T19:00:15Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20086#issuecomment-287942",
    "user": "https://github.com/videlec"
}

comment:62

The code provided is far to be working on any UFD as you have to factor the unit! But we know for sure that it will work for polynomial rings whose base ring provides a nth_root method. It would make sense to move the code to generic polynomial. As there is not a lot of examples of UFD in Sage it is hard to say that this code is useful for general UFD.

archive/issue_comments_287943.json:

{
    "body": "<div id=\"comment:63\" align=\"right\">comment:63</div>\n\nReplying to [@videlec](#comment%3A62):\n> The code provided is far to be working on any UFD as you have to factor the unit! \n\nI might still be thinking too much of polynomial rings when thinking of UFDs. ;-)\n\n> But we know for sure that it will work for polynomial rings whose base ring provides a `nth_root` method. It would make sense to move the code to generic polynomial. \n\nThe argument regarding the unit and the fact that there are not that much (exotic) UFDs implemented in Sage convice me; thanks for the clarification! :-) I'll move the code.",
    "created_at": "2016-05-02T19:08:06Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20086#issuecomment-287943",
    "user": "https://github.com/behackl"
}

comment:63

Replying to @videlec:

The code provided is far to be working on any UFD as you have to factor the unit!

I might still be thinking too much of polynomial rings when thinking of UFDs. ;-)

But we know for sure that it will work for polynomial rings whose base ring provides a nth_root method. It would make sense to move the code to generic polynomial.

The argument regarding the unit and the fact that there are not that much (exotic) UFDs implemented in Sage convice me; thanks for the clarification! :-) I'll move the code.

archive/issue_events_281161.json:

{
    "actor": "https://github.com/behackl",
    "created_at": "2016-05-02T19:08:06Z",
    "event": "unlabeled",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "label": "https://github.com/sagemath/sage/labels/needs%20info",
    "label_color": "ffff00",
    "label_name": "needs info",
    "label_text_color": "ffffff",
    "type": "issue_event",
    "url": "https://github.com/sagemath/sage/issues/20086#event-281161"
}

archive/issue_events_281162.json:

{
    "actor": "https://github.com/behackl",
    "created_at": "2016-05-02T19:08:06Z",
    "event": "labeled",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "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/20086#event-281162"
}

archive/issue_comments_287944.json:

{
    "body": "<div id=\"comment:64\" align=\"right\">comment:64</div>\n\nMoving the code to `sage.rings.polynomial.polynomial_element.Polynomial` only provides `nth_root` for univariate polynomial rings, as it seems. The doctests for multivariate rings fail with\n\n```\nAttributeError: 'sage.rings.polynomial.multi_polynomial_libsingular.MPolynomial_libsingular' object has no attribute 'nth_root'\n```\n\nI'm not a particularly big fried of copying the code to `sage.rings.polynomial.multi_polynomial_element.MPolynomial_element` as well. The univariate and multivariate polynomial ring elements inherit from CommutativeAlgebraelement and CommutativeRingElement, respectively---so implementing it in a superclass is not possible.\n\nIdeas?",
    "created_at": "2016-05-02T19:33:08Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20086#issuecomment-287944",
    "user": "https://github.com/behackl"
}

comment:64

Moving the code to sage.rings.polynomial.polynomial_element.Polynomial only provides nth_root for univariate polynomial rings, as it seems. The doctests for multivariate rings fail with

AttributeError: 'sage.rings.polynomial.multi_polynomial_libsingular.MPolynomial_libsingular' object has no attribute 'nth_root'

I'm not a particularly big fried of copying the code to sage.rings.polynomial.multi_polynomial_element.MPolynomial_element as well. The univariate and multivariate polynomial ring elements inherit from CommutativeAlgebraelement and CommutativeRingElement, respectively---so implementing it in a superclass is not possible.

Ideas?

archive/issue_comments_287945.json:

{
    "body": "<div id=\"comment:65\" align=\"right\">comment:65</div>\n\nReplying to [@behackl](#comment%3A64):\n> Ideas?\n\n- Leave the code in `categories.unique_factorization_domain`, raise a `NotImplementedError` ...\n  - ... if something goes wrong (current behavior), or \n  - ... when the calling parent is not a polynomial ring (that seems relatively restrictive to me)\n- Move it to either univariate or multivariate polynomials, call the same method from the other one (I'm thinking of something like `nth_root = sage.rings.polynomial.polynomial_element.nth_root` or so...)\n- Move it to both univaraite and multivariate polynomial rings\n\nI'll try to do the second one and push the branch again, let me know if you feel that there is a better solution.",
    "created_at": "2016-05-03T07:42:54Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20086#issuecomment-287945",
    "user": "https://github.com/behackl"
}

comment:65

Replying to @behackl:

Ideas?

Leave the code in categories.unique_factorization_domain, raise a NotImplementedError ...
- ... if something goes wrong (current behavior), or
- ... when the calling parent is not a polynomial ring (that seems relatively restrictive to me)
Move it to either univariate or multivariate polynomials, call the same method from the other one (I'm thinking of something like nth_root = sage.rings.polynomial.polynomial_element.nth_root or so...)
Move it to both univaraite and multivariate polynomial rings

I'll try to do the second one and push the branch again, let me know if you feel that there is a better solution.

archive/issue_comments_287946.json:

{
    "body": "<div id=\"comment:66\"></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/7940c9f126743aadb02ef8325dd474f9986592ce\"><code>7940c9f</code></a></td><td><code>duplicate code for univariate/multivariate polynomial rings</code></td></tr></table>\n",
    "created_at": "2016-05-03T08:16:45Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20086#issuecomment-287946",
    "user": "https://github.com/sagetrac-git"
}

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

7940c9f duplicate code for univariate/multivariate polynomial rings

archive/issue_comments_287947.json:

{
    "body": "Changed commit from **[`6f91df9`](https://github.com/sagemath/sagetrac-mirror/commit/6f91df97a81fa094c04583314ad38cd5fd199cdb)** to **[`7940c9f`](https://github.com/sagemath/sagetrac-mirror/commit/7940c9f126743aadb02ef8325dd474f9986592ce)**",
    "created_at": "2016-05-03T08:16:45Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20086#issuecomment-287947",
    "user": "https://github.com/sagetrac-git"
}

Changed commit from 6f91df9 to 7940c9f

archive/issue_events_281163.json:

{
    "actor": "https://github.com/behackl",
    "created_at": "2016-05-03T08:20:46Z",
    "event": "unlabeled",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "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/20086#event-281163"
}

archive/issue_events_281164.json:

{
    "actor": "https://github.com/behackl",
    "created_at": "2016-05-03T08:20:46Z",
    "event": "labeled",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "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/20086#event-281164"
}

archive/issue_comments_287948.json:

{
    "body": "<div id=\"comment:67\" align=\"right\">comment:67</div>\n\nMy attempt to implement the second strategy failed spectacularly with\n\n```\nTypeError: descriptor 'nth_root' for 'sage.rings.polynomial.polynomial_element.Polynomial' objects doesn't apply to 'sage.rings.polynomial.multi_polynomial_libsingular.MPolynomial_libsingular' object\n```\n\nso I've duplicated the code and adapted the doctests a bit.\n\nI'm not particularly happy with this solution, but I don't see a better one*. Suggestions are very welcome. Back to `needs_review` again...\n\n*EDIT: that doesn't live in `categories.unique_factorization_domains`. There seems to be nothing sufficiently in common between the univariate and multivariate polynomial rings.",
    "created_at": "2016-05-03T08:20:46Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20086#issuecomment-287948",
    "user": "https://github.com/behackl"
}

comment:67

My attempt to implement the second strategy failed spectacularly with

TypeError: descriptor 'nth_root' for 'sage.rings.polynomial.polynomial_element.Polynomial' objects doesn't apply to 'sage.rings.polynomial.multi_polynomial_libsingular.MPolynomial_libsingular' object

so I've duplicated the code and adapted the doctests a bit.

I'm not particularly happy with this solution, but I don't see a better one*. Suggestions are very welcome. Back to needs_review again...

*EDIT: that doesn't live in categories.unique_factorization_domains. There seems to be nothing sufficiently in common between the univariate and multivariate polynomial rings.

archive/issue_comments_287949.json:

{
    "body": "Changed commit from **[`7940c9f`](https://github.com/sagemath/sagetrac-mirror/commit/7940c9f126743aadb02ef8325dd474f9986592ce)** to **[`0fc0744`](https://github.com/sagemath/sagetrac-mirror/commit/0fc07442120a84047511625a8fe4bbd1cef5bb60)**",
    "created_at": "2016-05-05T18:09:15Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20086#issuecomment-287949",
    "user": "https://github.com/sagetrac-git"
}

Changed commit from 7940c9f to 0fc0744

archive/issue_comments_287950.json:

{
    "body": "<div id=\"comment:68\"></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/0fc07442120a84047511625a8fe4bbd1cef5bb60\"><code>0fc0744</code></a></td><td><code>add remark regarding duplicated code</code></td></tr></table>\n",
    "created_at": "2016-05-05T18:09:15Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20086#issuecomment-287950",
    "user": "https://github.com/sagetrac-git"
}

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

0fc0744 add remark regarding duplicated code

archive/issue_comments_287951.json:

{
    "body": "Changed commit from **[`0fc0744`](https://github.com/sagemath/sagetrac-mirror/commit/0fc07442120a84047511625a8fe4bbd1cef5bb60)** to **[`8cef2c7`](https://github.com/sagemath/sagetrac-mirror/commit/8cef2c7bce3fc7379f95d6c7c3bd6fb21e64c0ab)**",
    "created_at": "2016-05-05T18:13:14Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20086#issuecomment-287951",
    "user": "https://github.com/sagetrac-git"
}

Changed commit from 0fc0744 to 8cef2c7

archive/issue_comments_287952.json:

{
    "body": "<div id=\"comment:69\"></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/8cef2c7bce3fc7379f95d6c7c3bd6fb21e64c0ab\"><code>8cef2c7</code></a></td><td><code>fix a doctest in polynomial_quotient_ring.py</code></td></tr></table>\n",
    "created_at": "2016-05-05T18:13:14Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20086#issuecomment-287952",
    "user": "https://github.com/sagetrac-git"
}

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

8cef2c7 fix a doctest in polynomial_quotient_ring.py

archive/issue_comments_287953.json:

{
    "body": "<div id=\"comment:70\" align=\"right\">comment:70</div>\n\nI've added comments that there is a duplicated version of the code. Also, I've fixed the failing doctest.\n\nEven though we are too late for `7.2`, I'd still greatly appreciate if someone could review my last few changes. The code itself is already reviewed.",
    "created_at": "2016-05-05T18:24:38Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20086#issuecomment-287953",
    "user": "https://github.com/behackl"
}

comment:70

I've added comments that there is a duplicated version of the code. Also, I've fixed the failing doctest.

Even though we are too late for 7.2, I'd still greatly appreciate if someone could review my last few changes. The code itself is already reviewed.

archive/issue_comments_287954.json:

{
    "body": "<div id=\"comment:71\" align=\"right\">comment:71</div>\n\nSince now the code is in the polynomial ring you can simplify it a lot. Just remove all of\n\n```\nu = f.unit()\nif u.is_one():\n    ...\nelse:\n    ....\n```\nand do\n\n```\nu = self.base_ring()(f.unit())\ntry:\n    u.nth_root(n)\nexcept AttributeError:\n    raise NotImplementedError\n```",
    "created_at": "2016-05-06T11:54:52Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20086#issuecomment-287954",
    "user": "https://github.com/videlec"
}

comment:71

Since now the code is in the polynomial ring you can simplify it a lot. Just remove all of

u = f.unit()
if u.is_one():
    ...
else:
    ....

and do

u = self.base_ring()(f.unit())
try:
    u.nth_root(n)
except AttributeError:
    raise NotImplementedError

archive/issue_comments_287955.json:

{
    "body": "<div id=\"comment:72\" align=\"right\">comment:72</div>\n\nAnd if the degree is not divisible by `n` you already know that there is no n-th root (in multivariate case, degree=\"sum of degrees of monomials\"). This is very cheap to test.",
    "created_at": "2016-05-06T12:10:12Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20086#issuecomment-287955",
    "user": "https://github.com/videlec"
}

comment:72

And if the degree is not divisible by n you already know that there is no n-th root (in multivariate case, degree="sum of degrees of monomials"). This is very cheap to test.

archive/issue_comments_287956.json:

{
    "body": "Changed commit from **[`8cef2c7`](https://github.com/sagemath/sagetrac-mirror/commit/8cef2c7bce3fc7379f95d6c7c3bd6fb21e64c0ab)** to **[`753462d`](https://github.com/sagemath/sagetrac-mirror/commit/753462d07465a87797609856bec781797da9a6b1)**",
    "created_at": "2016-05-06T13:25:21Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20086#issuecomment-287956",
    "user": "https://github.com/sagetrac-git"
}

Changed commit from 8cef2c7 to 753462d

archive/issue_comments_287957.json:

{
    "body": "<div id=\"comment:73\"></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/753462d07465a87797609856bec781797da9a6b1\"><code>753462d</code></a></td><td><code>more refactoring after moving code to rings.polynomial</code></td></tr></table>\n",
    "created_at": "2016-05-06T13:25:21Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20086#issuecomment-287957",
    "user": "https://github.com/sagetrac-git"
}

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

753462d more refactoring after moving code to rings.polynomial

archive/issue_comments_287958.json:

{
    "body": "<div id=\"comment:74\" align=\"right\">comment:74</div>\n\nReplying to [@sagetrac-git](#comment%3A73):\n> Branch pushed to git repo; I updated commit sha1. New commits:\n<table><tr><td><a href=\"https://github.com/sagemath/sagetrac-mirror/commit/753462d07465a87797609856bec781797da9a6b1\"><code>753462d</code></a></td><td><code>more refactoring after moving code to rings.polynomial</code></td></tr></table>\n\nSimplified the code and added the additional check for `self.degree() % n`.",
    "created_at": "2016-05-06T13:26:38Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20086#issuecomment-287958",
    "user": "https://github.com/behackl"
}

comment:74

Replying to @sagetrac-git:

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

753462d more refactoring after moving code to rings.polynomial

Simplified the code and added the additional check for self.degree() % n.

archive/issue_comments_287959.json:

{
    "body": "<div id=\"comment:75\" align=\"right\">comment:75</div>\n\nPerhaps don't bother getting a grammatically correct ordinal. Note that the current code doesn't get it correct anyway: It's 21st, 22nd, 23rd.\n\nBetter to formulate the error message in a way that doesn't depend on correct ordinal spelling, e.g.\n`ValueError(\"(%s)^(1/%s) does not lie in ring\"%(f,n))`\n\nConcerning further optimization: testing degree is of course a worthwhile cheap trick. A follow-up ticket should probably use square-free factorization; something along the lines:\n\n- if the characteristic p divides n then first check that the polynomial only has p-th powers of the variables in it. Take p-th root (i.e., replace variables and take p-th root of coefficients) and take (n/p)-th root of resulting polynomial\n\n- In characteristic 0 then take g=f/GCD(f,f.derivative()), check that `g^n` divides `f`, take n-th root of `f/(g^n)`, multiply by g and return that.  \n\nIn positive characteristic we may first need to take all p-th powers out.",
    "created_at": "2016-05-07T06:19:05Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20086#issuecomment-287959",
    "user": "https://github.com/nbruin"
}

comment:75

Perhaps don't bother getting a grammatically correct ordinal. Note that the current code doesn't get it correct anyway: It's 21st, 22nd, 23rd.

Better to formulate the error message in a way that doesn't depend on correct ordinal spelling, e.g. ValueError("(%s)^(1/%s) does not lie in ring"%(f,n))

Concerning further optimization: testing degree is of course a worthwhile cheap trick. A follow-up ticket should probably use square-free factorization; something along the lines:

if the characteristic p divides n then first check that the polynomial only has p-th powers of the variables in it. Take p-th root (i.e., replace variables and take p-th root of coefficients) and take (n/p)-th root of resulting polynomial
In characteristic 0 then take g=f/GCD(f,f.derivative()), check that g^n divides f, take n-th root of f/(g^n), multiply by g and return that.

In positive characteristic we may first need to take all p-th powers out.

archive/issue_comments_287960.json:

{
    "body": "Changed commit from **[`753462d`](https://github.com/sagemath/sagetrac-mirror/commit/753462d07465a87797609856bec781797da9a6b1)** to **[`e56adf4`](https://github.com/sagemath/sagetrac-mirror/commit/e56adf4325379d05441efb4dd9487c2cc7904ddc)**",
    "created_at": "2016-05-07T08:19:49Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20086#issuecomment-287960",
    "user": "https://github.com/sagetrac-git"
}

Changed commit from 753462d to e56adf4

archive/issue_comments_287961.json:

{
    "body": "<div id=\"comment:76\"></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/e56adf4325379d05441efb4dd9487c2cc7904ddc\"><code>e56adf4</code></a></td><td><code>fix ordinals</code></td></tr></table>\n",
    "created_at": "2016-05-07T08:19:49Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20086#issuecomment-287961",
    "user": "https://github.com/sagetrac-git"
}

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

e56adf4 fix ordinals

archive/issue_comments_287962.json:

{
    "body": "<div id=\"comment:77\" align=\"right\">comment:77</div>\n\nReplying to [@nbruin](#comment%3A75):\n> Perhaps don't bother getting a grammatically correct ordinal. Note that the current code doesn't get it correct anyway: It's 21st, 22nd, 23rd.\n> \n> Better to formulate the error message in a way that doesn't depend on correct ordinal spelling, e.g.\n> `ValueError(\"(%s)^(1/%s) does not lie in ring\"%(f,n))`\n> \n\nI do like the version with explicit ordinals in the errors more, so I fixed them. :-)\n\n\n\n> Concerning further optimization: testing degree is of course a worthwhile cheap trick. A follow-up ticket should probably use square-free factorization; something along the lines:\n> \n> - if the characteristic p divides n then first check that the polynomial only has p-th powers of the variables in it. Take p-th root (i.e., replace variables and take p-th root of coefficients) and take (n/p)-th root of resulting polynomial\n> \n> - In characteristic 0 then take g=f/GCD(f,f.derivative()), check that `g^n` divides `f`, take n-th root of `f/(g^n)`, multiply by g and return that.  \n> \n> In positive characteristic we may first need to take all p-th powers out.\n\nOf course, there is always potential to improve the performance. However, like you said, this should happen in a follow-up ticket.",
    "created_at": "2016-05-07T08:36:28Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20086#issuecomment-287962",
    "user": "https://github.com/behackl"
}

comment:77

Replying to @nbruin:

Perhaps don't bother getting a grammatically correct ordinal. Note that the current code doesn't get it correct anyway: It's 21st, 22nd, 23rd.

Better to formulate the error message in a way that doesn't depend on correct ordinal spelling, e.g. ValueError("(%s)^(1/%s) does not lie in ring"%(f,n))

I do like the version with explicit ordinals in the errors more, so I fixed them. :-)

Concerning further optimization: testing degree is of course a worthwhile cheap trick. A follow-up ticket should probably use square-free factorization; something along the lines:

if the characteristic p divides n then first check that the polynomial only has p-th powers of the variables in it. Take p-th root (i.e., replace variables and take p-th root of coefficients) and take (n/p)-th root of resulting polynomial

In characteristic 0 then take g=f/GCD(f,f.derivative()), check that g^n divides f, take n-th root of f/(g^n), multiply by g and return that.

In positive characteristic we may first need to take all p-th powers out.

Of course, there is always potential to improve the performance. However, like you said, this should happen in a follow-up ticket.

archive/issue_comments_287963.json:

{
    "body": "<div id=\"comment:78\" align=\"right\">comment:78</div>\n\nReplying to [@behackl](#comment%3A77):\n> Replying to [@nbruin](#comment%3A75):\n> > Perhaps don't bother getting a grammatically correct ordinal. Note that the current code doesn't get it correct anyway: It's 21st, 22nd, 23rd.\n> > \n> > Better to formulate the error message in a way that doesn't depend on correct ordinal spelling, e.g.\n> > `ValueError(\"(%s)^(1/%s) does not lie in ring\"%(f,n))`\n> > \n> \n> \n> I do like the version with explicit ordinals in the errors more, so I fixed them. :-)\n\nHaving the following code executed each time the function run is useless\n\n```\n        if 10 <= n % 100 < 20:\n            postfix = 'th'\n        else:\n            postfix = {1:'st', 2:'nd', 3:'rd'}.get(n % 10, 'th')\n```\nMoreover, having these four lines potentially anywhere in Sage because you like is not the way to go. If you really think it is better this way, then implement a function `ordinal_str` that would *really* be doctested.\n\nIn the error message proposed by Nils there was the explicit mention of the parent which is a good thing.",
    "created_at": "2016-05-07T12:27:19Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20086#issuecomment-287963",
    "user": "https://github.com/videlec"
}

comment:78

Replying to @behackl:

Replying to @nbruin:

Perhaps don't bother getting a grammatically correct ordinal. Note that the current code doesn't get it correct anyway: It's 21st, 22nd, 23rd.

Better to formulate the error message in a way that doesn't depend on correct ordinal spelling, e.g. ValueError("(%s)^(1/%s) does not lie in ring"%(f,n))

I do like the version with explicit ordinals in the errors more, so I fixed them. :-)

Having the following code executed each time the function run is useless

        if 10 <= n % 100 < 20:
            postfix = 'th'
        else:
            postfix = {1:'st', 2:'nd', 3:'rd'}.get(n % 10, 'th')

Moreover, having these four lines potentially anywhere in Sage because you like is not the way to go. If you really think it is better this way, then implement a function ordinal_str that would really be doctested.

In the error message proposed by Nils there was the explicit mention of the parent which is a good thing.

archive/issue_comments_287964.json:

{
    "body": "<div id=\"comment:79\"></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/e8adfb10e935659d9701aa69b888571b569e222a\"><code>e8adfb1</code></a></td><td><code>remove ordinals, improve error messages</code></td></tr></table>\n",
    "created_at": "2016-05-07T13:14:23Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20086#issuecomment-287964",
    "user": "https://github.com/sagetrac-git"
}

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

e8adfb1 remove ordinals, improve error messages

archive/issue_comments_287965.json:

{
    "body": "Changed commit from **[`e56adf4`](https://github.com/sagemath/sagetrac-mirror/commit/e56adf4325379d05441efb4dd9487c2cc7904ddc)** to **[`e8adfb1`](https://github.com/sagemath/sagetrac-mirror/commit/e8adfb10e935659d9701aa69b888571b569e222a)**",
    "created_at": "2016-05-07T13:14:23Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20086#issuecomment-287965",
    "user": "https://github.com/sagetrac-git"
}

Changed commit from e56adf4 to e8adfb1

archive/issue_comments_287966.json:

{
    "body": "<div id=\"comment:80\" align=\"right\">comment:80</div>\n\n\n```\nsage: R.<x> = ZZ[]\nsage: parent(R.one().nth_root(3))\nInteger Ring\n```",
    "created_at": "2016-05-07T13:22:05Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20086#issuecomment-287966",
    "user": "https://github.com/videlec"
}

comment:80

sage: R.<x> = ZZ[]
sage: parent(R.one().nth_root(3))
Integer Ring

archive/issue_events_281165.json:

{
    "actor": "https://github.com/videlec",
    "created_at": "2016-05-07T13:22:05Z",
    "event": "unlabeled",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "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/20086#event-281165"
}

archive/issue_events_281166.json:

{
    "actor": "https://github.com/videlec",
    "created_at": "2016-05-07T13:22:05Z",
    "event": "labeled",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "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/20086#event-281166"
}

archive/issue_comments_287967.json:

{
    "body": "<div id=\"comment:81\" align=\"right\">comment:81</div>\n\nReplying to [@videlec](#comment%3A80):\n> \n> ```\n> sage: R.<x> = ZZ[]\n> sage: parent(R.one().nth_root(3))\n> Integer Ring\n> ```\n\nFixed, will push in a moment.",
    "created_at": "2016-05-07T13:25:52Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20086#issuecomment-287967",
    "user": "https://github.com/behackl"
}

comment:81

Replying to @videlec:

sage: R.<x> = ZZ[]
sage: parent(R.one().nth_root(3))
Integer Ring

Fixed, will push in a moment.

archive/issue_comments_287968.json:

{
    "body": "Changed commit from **[`e8adfb1`](https://github.com/sagemath/sagetrac-mirror/commit/e8adfb10e935659d9701aa69b888571b569e222a)** to **[`2c46e08`](https://github.com/sagemath/sagetrac-mirror/commit/2c46e087a34666657dee8cb2f168a08d8afad488)**",
    "created_at": "2016-05-07T13:31:42Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20086#issuecomment-287968",
    "user": "https://github.com/sagetrac-git"
}

Changed commit from e8adfb1 to 2c46e08

archive/issue_comments_287969.json:

{
    "body": "<div id=\"comment:82\"></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/2c46e087a34666657dee8cb2f168a08d8afad488\"><code>2c46e08</code></a></td><td><code>return one from original parent, not base ring</code></td></tr></table>\n",
    "created_at": "2016-05-07T13:31:42Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20086#issuecomment-287969",
    "user": "https://github.com/sagetrac-git"
}

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

2c46e08 return one from original parent, not base ring

archive/issue_events_281167.json:

{
    "actor": "https://github.com/behackl",
    "created_at": "2016-05-07T13:36:10Z",
    "event": "unlabeled",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "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/20086#event-281167"
}

archive/issue_events_281168.json:

{
    "actor": "https://github.com/behackl",
    "created_at": "2016-05-07T13:36:10Z",
    "event": "labeled",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "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/20086#event-281168"
}

archive/issue_comments_287970.json:

{
    "body": "<div id=\"comment:84\" align=\"right\">comment:84</div>\n\nPatchbot reports doctest failure due to the change in the error message.",
    "created_at": "2016-05-07T15:36:27Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20086#issuecomment-287970",
    "user": "https://github.com/videlec"
}

comment:84

Patchbot reports doctest failure due to the change in the error message.

archive/issue_events_281169.json:

{
    "actor": "https://github.com/videlec",
    "created_at": "2016-05-08T01:52:24Z",
    "event": "unlabeled",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "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/20086#event-281169"
}

archive/issue_events_281170.json:

{
    "actor": "https://github.com/videlec",
    "created_at": "2016-05-08T01:52:24Z",
    "event": "labeled",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "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/20086#event-281170"
}

archive/issue_comments_287971.json:

{
    "body": "<div id=\"comment:85\" align=\"right\">comment:85</div>\n\nI have an implementation for a much faster implementation of n-th root at #20571.",
    "created_at": "2016-05-08T01:52:24Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20086#issuecomment-287971",
    "user": "https://github.com/videlec"
}

comment:85

I have an implementation for a much faster implementation of n-th root at #20571.

archive/issue_comments_287972.json:

{
    "body": "Description changed:\n``````diff\n--- \n+++ \n@@ -24,3 +24,5 @@\n since its coefficient 1 cannot be taken to this exponent.\n > *previous* TypeError: rational is not an integer\n ```\n+\n+follow up: #20086\n``````\n",
    "created_at": "2016-05-08T01:52:24Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20086#issuecomment-287972",
    "user": "https://github.com/videlec"
}

Description changed:

--- 
+++ 
@@ -24,3 +24,5 @@
 since its coefficient 1 cannot be taken to this exponent.
 > *previous* TypeError: rational is not an integer
 ```
+
+follow up: #20086

archive/issue_comments_287973.json:

{
    "body": "Description changed:\n``````diff\n--- \n+++ \n@@ -25,4 +25,4 @@\n > *previous* TypeError: rational is not an integer\n ```\n \n-follow up: #20086\n+follow up: #20751\n``````\n",
    "created_at": "2016-05-08T01:52:40Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20086#issuecomment-287973",
    "user": "https://github.com/videlec"
}

Description changed:

--- 
+++ 
@@ -25,4 +25,4 @@
 > *previous* TypeError: rational is not an integer
 ```
 
-follow up: #20086
+follow up: #20751

archive/issue_comments_287974.json:

{
    "body": "Description changed:\n``````diff\n--- \n+++ \n@@ -25,4 +25,4 @@\n > *previous* TypeError: rational is not an integer\n ```\n \n-follow up: #20751\n+follow up: #20571\n``````\n",
    "created_at": "2016-05-08T01:53:11Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20086#issuecomment-287974",
    "user": "https://github.com/videlec"
}

Description changed:

--- 
+++ 
@@ -25,4 +25,4 @@
 > *previous* TypeError: rational is not an integer
 ```
 
-follow up: #20751
+follow up: #20571

archive/issue_comments_287975.json:

{
    "body": "<div id=\"comment:88\" align=\"right\">comment:88</div>\n\nReplying to [@videlec](#comment%3A84):\n> Patchbot reports doctest failure due to the change in the error message.\n\nIndeed, and this can be fixed straightforward. However, there is (once again ...) a slight inconvenience: there are superfluous parentheses in, e.g.\n\n```\nsage: P.<x> = ZZ[]\nsage: P(4).nth_root(3)\nTraceback (most recent call last):\n...\nValueError: (4)^(1/3) does not lie in ...\n```\n\nor\n\n```\nsage: x.nth_root(3)\nTraceback (most recent call last):\n...\nValueError: (x)^(1/3) does not lie in ...\n```\n\nShould we live with that? Fixing it would require querying something like `self.is_constant()` and `self in self.parent().gens()` or so.\n\nEDIT: `self.is_constant()` would not work:\n\n```\nsage: P.<x> = ZZ[]\nsage: Q.<y> = P[]\nsage: Q(x+1).is_constant()\nTrue\n```",
    "created_at": "2016-05-08T07:05:37Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20086#issuecomment-287975",
    "user": "https://github.com/behackl"
}

comment:88

Replying to @videlec:

Patchbot reports doctest failure due to the change in the error message.

Indeed, and this can be fixed straightforward. However, there is (once again ...) a slight inconvenience: there are superfluous parentheses in, e.g.

sage: P.<x> = ZZ[]
sage: P(4).nth_root(3)
Traceback (most recent call last):
...
ValueError: (4)^(1/3) does not lie in ...

or

sage: x.nth_root(3)
Traceback (most recent call last):
...
ValueError: (x)^(1/3) does not lie in ...

Should we live with that? Fixing it would require querying something like self.is_constant() and self in self.parent().gens() or so.

EDIT: self.is_constant() would not work:

sage: P.<x> = ZZ[]
sage: Q.<y> = P[]
sage: Q(x+1).is_constant()
True

archive/issue_comments_287976.json:

{
    "body": "<div id=\"comment:89\" align=\"right\">comment:89</div>\n\nI would think most people would prioritize #20571 over such typographical issues, so I wouldn't sweat too hard over parentheses at this point. Another issue: while it's nice to have informative error messages, error strings that cost a lot to be constructed only to be caught by an \"except\" statement can mean a performance penalty. This is more an issue in python than in many other languages because there's a culture in python (and perhaps even more so in some parts of sage) to use exceptions for regular program flow control, not just for error conditions.",
    "created_at": "2016-05-08T20:58:59Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20086#issuecomment-287976",
    "user": "https://github.com/nbruin"
}

comment:89

I would think most people would prioritize #20571 over such typographical issues, so I wouldn't sweat too hard over parentheses at this point. Another issue: while it's nice to have informative error messages, error strings that cost a lot to be constructed only to be caught by an "except" statement can mean a performance penalty. This is more an issue in python than in many other languages because there's a culture in python (and perhaps even more so in some parts of sage) to use exceptions for regular program flow control, not just for error conditions.

archive/issue_comments_287977.json:

{
    "body": "<div id=\"comment:90\"></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/5fc1027e43d601a791568b22f12ec70957c47519\"><code>5fc1027</code></a></td><td><code>fix doctests</code></td></tr></table>\n",
    "created_at": "2016-05-08T22:56:50Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20086#issuecomment-287977",
    "user": "https://github.com/sagetrac-git"
}

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

5fc1027 fix doctests

archive/issue_comments_287978.json:

{
    "body": "Changed commit from **[`2c46e08`](https://github.com/sagemath/sagetrac-mirror/commit/2c46e087a34666657dee8cb2f168a08d8afad488)** to **[`5fc1027`](https://github.com/sagemath/sagetrac-mirror/commit/5fc1027e43d601a791568b22f12ec70957c47519)**",
    "created_at": "2016-05-08T22:56:50Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20086#issuecomment-287978",
    "user": "https://github.com/sagetrac-git"
}

Changed commit from 2c46e08 to 5fc1027

archive/issue_comments_287979.json:

{
    "body": "<div id=\"comment:91\" align=\"right\">comment:91</div>\n\nReplying to [@nbruin](#comment%3A89):\n> I would think most people would prioritize #20571 over such typographical issues, so I wouldn't sweat too hard over parentheses at this point. Another issue: while it's nice to have informative error messages, error strings that cost a lot to be constructed only to be caught by an \"except\" statement can mean a performance penalty. This is more an issue in python than in many other languages because there's a culture in python (and perhaps even more so in some parts of sage) to use exceptions for regular program flow control, not just for error conditions.\n\nYes, that's my feeling too. While it is somehow unfortunate, I can very well live with it. \n\nI've fixed the doctests and tested the entire `src/sage/rings/polynomial`-directory; let's see what the patchbot thinks.",
    "created_at": "2016-05-08T23:00:41Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20086#issuecomment-287979",
    "user": "https://github.com/behackl"
}

comment:91

Replying to @nbruin:

I would think most people would prioritize #20571 over such typographical issues, so I wouldn't sweat too hard over parentheses at this point. Another issue: while it's nice to have informative error messages, error strings that cost a lot to be constructed only to be caught by an "except" statement can mean a performance penalty. This is more an issue in python than in many other languages because there's a culture in python (and perhaps even more so in some parts of sage) to use exceptions for regular program flow control, not just for error conditions.

Yes, that's my feeling too. While it is somehow unfortunate, I can very well live with it.

I've fixed the doctests and tested the entire src/sage/rings/polynomial-directory; let's see what the patchbot thinks.

archive/issue_events_281171.json:

{
    "actor": "https://github.com/behackl",
    "created_at": "2016-05-09T15:24:08Z",
    "event": "unlabeled",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "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/20086#event-281171"
}

archive/issue_events_281172.json:

{
    "actor": "https://github.com/behackl",
    "created_at": "2016-05-09T15:24:08Z",
    "event": "labeled",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "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/20086#event-281172"
}

archive/issue_comments_287980.json:

{
    "body": "<div id=\"comment:93\" align=\"right\">comment:93</div>\n\nLet's move to #20571.",
    "created_at": "2016-05-10T04:48:51Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20086#issuecomment-287980",
    "user": "https://github.com/videlec"
}

comment:93

Let's move to #20571.

archive/issue_events_281173.json:

{
    "actor": "https://github.com/videlec",
    "created_at": "2016-05-10T04:48:51Z",
    "event": "unlabeled",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "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/20086#event-281173"
}

archive/issue_events_281174.json:

{
    "actor": "https://github.com/videlec",
    "created_at": "2016-05-10T04:48:51Z",
    "event": "labeled",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "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/20086#event-281174"
}

archive/issue_events_281175.json:

{
    "actor": "https://github.com/vbraun",
    "created_at": "2016-05-17T07:16:35Z",
    "event": "unlabeled",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "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/20086#event-281175"
}

archive/issue_events_281176.json:

{
    "actor": "https://github.com/vbraun",
    "commit_id": "5145cd5f8dcdc2c7b242e2dd2d7566947ae3d2c5",
    "commit_repository": "https://github.com/sagemath/sage",
    "created_at": "2016-05-17T07:16:35Z",
    "event": "closed",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "type": "issue_event",
    "url": "https://github.com/sagemath/sage/issues/20086#event-281176"
}

archive/issue_comments_287981.json:

{
    "body": "Changed branch from **[public/20086](https://github.com/sagemath/sagetrac-mirror/tree/public/20086)** to **[`5fc1027`](https://github.com/sagemath/sagetrac-mirror/commit/5fc1027e43d601a791568b22f12ec70957c47519)**",
    "created_at": "2016-05-17T07:16:35Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/20086",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/20086#issuecomment-287981",
    "user": "https://github.com/vbraun"
}

Changed branch from public/20086 to 5fc1027

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

20086.md

20086.md

Issue 20086: rational powers in ZZ[X] and QQ[X]

Files

20086.md

Latest commit

History

20086.md

File metadata and controls

Issue 20086: rational powers in ZZ[X] and QQ[X]