Skip to content
This repository was archived by the owner on Jan 31, 2023. It is now read-only.

Latest commit

 

History

History
1429 lines (1081 loc) · 60.1 KB

18231.md

File metadata and controls

1429 lines (1081 loc) · 60.1 KB

Issue 18231: constructing matrices is very slow

archive/issues_017994.json:

{
    "assignees": [],
    "body": "<div id=\"comment:0\"></div>\n\nThe problem is in the code which looks like the following\n\n```\ndef test1():\n    K = QuadraticField(2)\n    for _ in range(100):\n        i = randint(2,3)\n        j = randint(2,3)\n        m = matrix(K, i, j, range(i*j))\n```\nIt causes the various matrix spaces to rebuilt almost each time! If we compare with\n\n```\ndef test2():\n    K = QuadraticField(2)\n    M = {(i,j): MatrixSpace(K,i,j) for i in range(2,4) for j in range(2,4)}\n    for _ in range(100):\n        i = randint(2,3)\n        j = randint(2,3)\n        m = M[i,j](range(i*j))\n```\nthen we got\n\n```\nsage: %timeit test1()\n10 loops, best of 3: 55 ms per loop\nsage: %timeit test2()\n100 loops, best of 3: 6.49 ms per loop\n```\n\nThis is one of the reasons why the creation of Polyhedron is so slow (see #18213). (This has now improved since the creation of the ticket)\n\nIn the branch we:\n- replace some old Cython code in `matrix_generic_dense` and `matrix0` and do some optimization\n- move the function `_get_matrix_class` in `MatrixSpace` out of the class (as it depends only on the base ring and the sparseness)\n- In the class `MatrixSpace` the argument `__matrix_class` is now `_matrix_class`. That way, it is possible in other part of Sage to optimize the creation of matrices by calling directly\n\n```\nsage: M = MatrixSpace(ZZ,3,2)\nsage: M._matrix_class([1,5,3,-1,2,3], coerce=False, copy=False)\n```\n- we simplify the main element constructor of matrix, namely `MatrixSpace.matrix` and gain a significant speedup.\n\nfollow up: #18258\n\nCC:  @nathanncohen\n\nComponent: **linear algebra**\n\n_Issue created by migration from https://trac.sagemath.org/ticket/18231_\n\n",
    "closed_at": "2022-02-25T10:34:15Z",
    "created_at": "2015-04-16T22:16:20Z",
    "labels": [
        "https://github.com/sagemath/sage/labels/bug",
        "https://github.com/sagemath/sage/labels/c%3A%20linear%20algebra",
        "https://github.com/sagemath/sage/labels/duplicate"
    ],
    "reactions": [],
    "repository": "https://github.com/sagemath/sage",
    "title": "constructing matrices is very slow",
    "type": "issue",
    "updated_at": "2022-02-25T10:34:15Z",
    "url": "https://github.com/sagemath/sage/issues/18231",
    "user": "https://github.com/videlec"
}

The problem is in the code which looks like the following

def test1():
    K = QuadraticField(2)
    for _ in range(100):
        i = randint(2,3)
        j = randint(2,3)
        m = matrix(K, i, j, range(i*j))

It causes the various matrix spaces to rebuilt almost each time! If we compare with

def test2():
    K = QuadraticField(2)
    M = {(i,j): MatrixSpace(K,i,j) for i in range(2,4) for j in range(2,4)}
    for _ in range(100):
        i = randint(2,3)
        j = randint(2,3)
        m = M[i,j](range(i*j))

then we got

sage: %timeit test1()
10 loops, best of 3: 55 ms per loop
sage: %timeit test2()
100 loops, best of 3: 6.49 ms per loop

This is one of the reasons why the creation of Polyhedron is so slow (see #18213). (This has now improved since the creation of the ticket)

In the branch we:

  • replace some old Cython code in matrix_generic_dense and matrix0 and do some optimization
  • move the function _get_matrix_class in MatrixSpace out of the class (as it depends only on the base ring and the sparseness)
  • In the class MatrixSpace the argument __matrix_class is now _matrix_class. That way, it is possible in other part of Sage to optimize the creation of matrices by calling directly
sage: M = MatrixSpace(ZZ,3,2)
sage: M._matrix_class([1,5,3,-1,2,3], coerce=False, copy=False)
  • we simplify the main element constructor of matrix, namely MatrixSpace.matrix and gain a significant speedup.

follow up: #18258

CC: @nathanncohen

Component: linear algebra

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


archive/issue_events_256843.json:

{
    "actor": "https://github.com/videlec",
    "created_at": "2015-04-16T22:16:20Z",
    "event": "milestoned",
    "issue": "https://github.com/sagemath/sage/issues/18231",
    "milestone_number": null,
    "milestone_title": "sage-6.7",
    "type": "issue_event",
    "url": "https://github.com/sagemath/sage/issues/18231#event-256843"
}

archive/issue_events_256844.json:

{
    "actor": "https://github.com/videlec",
    "created_at": "2015-04-16T22:16:20Z",
    "event": "labeled",
    "issue": "https://github.com/sagemath/sage/issues/18231",
    "label": "https://github.com/sagemath/sage/labels/c%3A%20linear%20algebra",
    "label_color": "0000ff",
    "label_name": "c: linear algebra",
    "label_text_color": "ffffff",
    "type": "issue_event",
    "url": "https://github.com/sagemath/sage/issues/18231#event-256844"
}

archive/issue_events_256845.json:

{
    "actor": "https://github.com/videlec",
    "created_at": "2015-04-16T22:16:20Z",
    "event": "labeled",
    "issue": "https://github.com/sagemath/sage/issues/18231",
    "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/18231#event-256845"
}

archive/issue_events_256846.json:

{
    "actor": "https://github.com/videlec",
    "created_at": "2015-04-16T22:16:20Z",
    "event": "labeled",
    "issue": "https://github.com/sagemath/sage/issues/18231",
    "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/18231#event-256846"
}

archive/issue_comments_249729.json:

{
    "body": "Author: **Vincent Delecroix**",
    "created_at": "2015-04-18T22:50:34Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/18231",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/18231#issuecomment-249729",
    "user": "https://github.com/videlec"
}

Author: Vincent Delecroix


archive/issue_comments_249730.json:

{
    "body": "Commit: **[`c1c7034`](https://github.com/sagemath/sagetrac-mirror/commit/c1c7034ed54c7493a7fedbf6fa9798f23946902c)**",
    "created_at": "2015-04-18T22:50:34Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/18231",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/18231#issuecomment-249730",
    "user": "https://github.com/videlec"
}

Commit: c1c7034


archive/issue_events_256847.json:

{
    "actor": "https://github.com/videlec",
    "created_at": "2015-04-18T22:50:34Z",
    "event": "labeled",
    "issue": "https://github.com/sagemath/sage/issues/18231",
    "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/18231#event-256847"
}

archive/issue_comments_249731.json:

{
    "body": "<div id=\"comment:1\" align=\"right\">comment:1</div>\n\nI only did some cleanup in `matrix_space.py` and `matrix_generic_dense.pyx`. And there is a substantial gain in speed! The tiny modification in #18213 to `NumberField.__cmp__` actually makes a much bigger difference.\n\nI did not solve the problem of the matrix space being rebuilt every time.\n\n---\nNew commits:\n<table><tr><td><a href=\"https://github.com/sagemath/sagetrac-mirror/commit/f6a87ed8ba412a58a9c8dd582ac716ac968fcf1b\"><code>f6a87ed</code></a></td><td><code>Trac 18231: better code in matrices</code></td></tr><tr><td><a href=\"https://github.com/sagemath/sagetrac-mirror/commit/d834354d719c55a88018c8c7bb0d6ab94542d771\"><code>d834354</code></a></td><td><code>Trac 18231: move _get_matrix_class out of the class</code></td></tr><tr><td><a href=\"https://github.com/sagemath/sagetrac-mirror/commit/d52a63da8ae80bb9be6d5e401568bb4cf4baef6c\"><code>d52a63d</code></a></td><td><code>Trac 18231: constructor + __matrix_class -> _matrix_class</code></td></tr><tr><td><a href=\"https://github.com/sagemath/sagetrac-mirror/commit/ecd6e2743f973b05632edc688bd7a99c68e22ab8\"><code>ecd6e27</code></a></td><td><code>Trac 18231: removed unused import</code></td></tr><tr><td><a href=\"https://github.com/sagemath/sagetrac-mirror/commit/a23d09c1d4420f0bef8271785225bd12d4ad05c1\"><code>a23d09c</code></a></td><td><code>Trac 18231: get rid of a one year old deprecation</code></td></tr><tr><td><a href=\"https://github.com/sagemath/sagetrac-mirror/commit/6e9aab1a94f9df0cdca30ba026639d48b550e24c\"><code>6e9aab1</code></a></td><td><code>Trac 18231: remove a useless function</code></td></tr><tr><td><a href=\"https://github.com/sagemath/sagetrac-mirror/commit/95c9dfe9d827b2b83bfa2eb76fcb5b44eb6726f3\"><code>95c9dfe</code></a></td><td><code>Trac 18231: set .__call__() as an alias of .matrix()</code></td></tr><tr><td><a href=\"https://github.com/sagemath/sagetrac-mirror/commit/6dbf25e277f2e73c6a917c4d41872e370f65d56f\"><code>6dbf25e</code></a></td><td><code>Trac 18231: rewrite .matrix()</code></td></tr><tr><td><a href=\"https://github.com/sagemath/sagetrac-mirror/commit/c1c7034ed54c7493a7fedbf6fa9798f23946902c\"><code>c1c7034</code></a></td><td><code>Trac 18231: fix dict_to_list</code></td></tr></table>\n",
    "created_at": "2015-04-18T22:50:34Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/18231",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/18231#issuecomment-249731",
    "user": "https://github.com/videlec"
}
comment:1

I only did some cleanup in matrix_space.py and matrix_generic_dense.pyx. And there is a substantial gain in speed! The tiny modification in #18213 to NumberField.__cmp__ actually makes a much bigger difference.

I did not solve the problem of the matrix space being rebuilt every time.


New commits:

f6a87edTrac 18231: better code in matrices
d834354Trac 18231: move _get_matrix_class out of the class
d52a63dTrac 18231: constructor + __matrix_class -> _matrix_class
ecd6e27Trac 18231: removed unused import
a23d09cTrac 18231: get rid of a one year old deprecation
6e9aab1Trac 18231: remove a useless function
95c9dfeTrac 18231: set .__call__() as an alias of .matrix()
6dbf25eTrac 18231: rewrite .matrix()
c1c7034Trac 18231: fix dict_to_list

archive/issue_comments_249732.json:

{
    "body": "Branch: **[u/vdelecroix/18231](https://github.com/sagemath/sagetrac-mirror/tree/u/vdelecroix/18231)**",
    "created_at": "2015-04-18T22:50:34Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/18231",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/18231#issuecomment-249732",
    "user": "https://github.com/videlec"
}

Branch: u/vdelecroix/18231


archive/issue_comments_249733.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/da310db9b3b4238121b1a1ad2c3d5e36a5e846b5\"><code>da310db</code></a></td><td><code>Trac 18231: typo</code></td></tr></table>\n",
    "created_at": "2015-04-18T22:56:10Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/18231",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/18231#issuecomment-249733",
    "user": "https://github.com/sagetrac-git"
}

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

da310dbTrac 18231: typo

archive/issue_comments_249734.json:

{
    "body": "Changed commit from **[`c1c7034`](https://github.com/sagemath/sagetrac-mirror/commit/c1c7034ed54c7493a7fedbf6fa9798f23946902c)** to **[`da310db`](https://github.com/sagemath/sagetrac-mirror/commit/da310db9b3b4238121b1a1ad2c3d5e36a5e846b5)**",
    "created_at": "2015-04-18T22:56:10Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/18231",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/18231#issuecomment-249734",
    "user": "https://github.com/sagetrac-git"
}

Changed commit from c1c7034 to da310db


archive/issue_events_256848.json:

{
    "actor": "https://github.com/nathanncohen",
    "created_at": "2015-04-19T06:41:15Z",
    "event": "unlabeled",
    "issue": "https://github.com/sagemath/sage/issues/18231",
    "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/18231#event-256848"
}

archive/issue_events_256849.json:

{
    "actor": "https://github.com/nathanncohen",
    "created_at": "2015-04-19T06:41:15Z",
    "event": "labeled",
    "issue": "https://github.com/sagemath/sage/issues/18231",
    "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/18231#event-256849"
}

archive/issue_comments_249735.json:

{
    "body": "<div id=\"comment:3\" align=\"right\">comment:3</div>\n\nHello Vincent,\n\nIn this branch you create functions, remove others, and change a lot of code. There is still a lot of guessing to do on the reviewer's part.\n\nIt would be really cool if you could be more verbose about what you do. Really.\n\nI began to read the first commit, and I have several more specific questions:\n\n- You modify a function which takes a `MonoidElement` as input, which you immeditely cast with `cdef Matrix right = _right`. Either it is a matrix and it should not read `MonoidElement` in the first place, or it is not a `Matrix` and you cannot cast it?..\n\n- This is cool and everything to gain miliseconds, but that's not a good way to write public code\n\n  ```\n  -        matrix.Matrix.__init__(self, parent)\n  +        # the four lines below avoid the following call\n  +        # matrix.Matrix.__init__(self, parent)\n  +        self._parent = parent\n  +        res._ncols  = parent.ncols()\n  +        res._nrows  = parent.nrows()\n  +        R = res._base_ring = parent.base_ring()\n  ```\n  If the main constructor does not do what you want ty to change it. What if somebody adds a new parameter to matrix? The code has to be copy/pasted in every single function that did not want to call `__init__`?\n\n- Why?\n\n  ```\n  \n       cdef set_unsafe(self, Py_ssize_t i, Py_ssize_t j, value):\n  -        Py_DECREF(<object>PyList_GET_ITEM(self._entries, i*self._ncols + j))\n  -        Py_INCREF(value)\n  -        PyList_SET_ITEM(self._entries, i*self._ncols + j, value)\n  +        self._entries[i*self._ncols + j] = value\n   \n       cdef get_unsafe(self, Py_ssize_t i, Py_ssize_t j):\n  -        return <object>PyList_GET_ITEM(self._entries, i*self._ncols + j)\n  +        return self._entries[i*self._ncols + j]\n  ```\n\nNathann",
    "created_at": "2015-04-19T06:41:15Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/18231",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/18231#issuecomment-249735",
    "user": "https://github.com/nathanncohen"
}
comment:3

Hello Vincent,

In this branch you create functions, remove others, and change a lot of code. There is still a lot of guessing to do on the reviewer's part.

It would be really cool if you could be more verbose about what you do. Really.

I began to read the first commit, and I have several more specific questions:

  • You modify a function which takes a MonoidElement as input, which you immeditely cast with cdef Matrix right = _right. Either it is a matrix and it should not read MonoidElement in the first place, or it is not a Matrix and you cannot cast it?..

  • This is cool and everything to gain miliseconds, but that's not a good way to write public code

    -        matrix.Matrix.__init__(self, parent)
    +        # the four lines below avoid the following call
    +        # matrix.Matrix.__init__(self, parent)
    +        self._parent = parent
    +        res._ncols  = parent.ncols()
    +        res._nrows  = parent.nrows()
    +        R = res._base_ring = parent.base_ring()
    

    If the main constructor does not do what you want ty to change it. What if somebody adds a new parameter to matrix? The code has to be copy/pasted in every single function that did not want to call __init__?

  • Why?

    
         cdef set_unsafe(self, Py_ssize_t i, Py_ssize_t j, value):
    -        Py_DECREF(<object>PyList_GET_ITEM(self._entries, i*self._ncols + j))
    -        Py_INCREF(value)
    -        PyList_SET_ITEM(self._entries, i*self._ncols + j, value)
    +        self._entries[i*self._ncols + j] = value
     
         cdef get_unsafe(self, Py_ssize_t i, Py_ssize_t j):
    -        return <object>PyList_GET_ITEM(self._entries, i*self._ncols + j)
    +        return self._entries[i*self._ncols + j]
    

Nathann


archive/issue_comments_249736.json:

{
    "body": "Description changed:\n``````diff\n--- \n+++ \n@@ -29,3 +29,14 @@\n ```\n \n This is one of the reason why the creation of Polyhedron is so slow (see #18213).\n+\n+In the branch we:\n+- replace some old Cython code in `matrix_generic_dense` and `matrix0` and do some optimization\n+- move the function `_get_matrix_class` in `MatrixSpace` out of the class (as it depends only on the base ring and the sparseness)\n+- In the class `MatrixSpace` the argument `__matrix_class` is now `_matrix_class`. That way, it is possible in other part of Sage to optimize the creation of matrices by calling directly\n+\n+```\n+sage: M = MatrixSpace(ZZ,3,2)\n+sage: M._matrix_class([1,5,3,-1,2,3], coerce=False, copy=False)\n+```\n+- we simplify the main element constructor of matrix, namely `MatrixSpace.matrix` and gain a significant speedup.\n``````\n",
    "created_at": "2015-04-19T09:52:48Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/18231",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/18231#issuecomment-249736",
    "user": "https://github.com/videlec"
}

Description changed:

--- 
+++ 
@@ -29,3 +29,14 @@
 ```
 
 This is one of the reason why the creation of Polyhedron is so slow (see #18213).
+
+In the branch we:
+- replace some old Cython code in `matrix_generic_dense` and `matrix0` and do some optimization
+- move the function `_get_matrix_class` in `MatrixSpace` out of the class (as it depends only on the base ring and the sparseness)
+- In the class `MatrixSpace` the argument `__matrix_class` is now `_matrix_class`. That way, it is possible in other part of Sage to optimize the creation of matrices by calling directly
+
+```
+sage: M = MatrixSpace(ZZ,3,2)
+sage: M._matrix_class([1,5,3,-1,2,3], coerce=False, copy=False)
+```
+- we simplify the main element constructor of matrix, namely `MatrixSpace.matrix` and gain a significant speedup.

archive/issue_events_256850.json:

{
    "actor": "https://github.com/videlec",
    "created_at": "2015-04-19T09:52:48Z",
    "event": "unlabeled",
    "issue": "https://github.com/sagemath/sage/issues/18231",
    "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/18231#event-256850"
}

archive/issue_events_256851.json:

{
    "actor": "https://github.com/videlec",
    "created_at": "2015-04-19T09:52:48Z",
    "event": "labeled",
    "issue": "https://github.com/sagemath/sage/issues/18231",
    "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/18231#event-256851"
}

archive/issue_comments_249737.json:

{
    "body": "<div id=\"comment:4\" align=\"right\">comment:4</div>\n\nHello Nathann,\n\nThanks for having a look.\n\nReplying to [@nathanncohen](#comment%3A3):\n> In this branch you create functions, remove others, and change a lot of code. There is still a lot of guessing to do on the reviewer's part.\n>\n>\n\n> It would be really cool if you could be more verbose about what you do. Really.\n\nI made a big effort to separate the commit at least! It is now in the description of the ticket.\n\n> I began to read the first commit, and I have several more specific questions:\n> \n> - You modify a function which takes a `MonoidElement` as input, which you immeditely cast with `cdef Matrix right = _right`. Either it is a matrix and it should not read `MonoidElement` in the first place, or it is not a `Matrix` and you cannot cast it?..\n\nNo choice. This is a cdef function defined in `structure.element.Element` and you can not change the signature in children classes. I thought it was more readable this way.\n\n> - This is cool and everything to gain miliseconds, but that's not a good way to write public code\n>   If the main constructor does not do what you want ty to change it. What if somebody adds a new parameter to matrix? The code has to be copy/pasted in every single function that did not want to call `__init__`?\n\nAll right. But then there is something wrong in the docstring of `matrix0.Matrix`\n\n```\n    def __init__(...):\n        ...\n        Subclasses of ``Matrix`` can safely skip calling\n        ``Matrix.__init__`` provided they take care of initializing these\n        attributes themselves.\n        ...\n```\n\n> - Why?\n\nAll this was just old Cython code at the time Cython was not able to understand `l[i]` when `l` is a `cdef list`.",
    "created_at": "2015-04-19T09:52:48Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/18231",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/18231#issuecomment-249737",
    "user": "https://github.com/videlec"
}
comment:4

Hello Nathann,

Thanks for having a look.

Replying to @nathanncohen:

In this branch you create functions, remove others, and change a lot of code. There is still a lot of guessing to do on the reviewer's part.

It would be really cool if you could be more verbose about what you do. Really.

I made a big effort to separate the commit at least! It is now in the description of the ticket.

I began to read the first commit, and I have several more specific questions:

  • You modify a function which takes a MonoidElement as input, which you immeditely cast with cdef Matrix right = _right. Either it is a matrix and it should not read MonoidElement in the first place, or it is not a Matrix and you cannot cast it?..

No choice. This is a cdef function defined in structure.element.Element and you can not change the signature in children classes. I thought it was more readable this way.

  • This is cool and everything to gain miliseconds, but that's not a good way to write public code If the main constructor does not do what you want ty to change it. What if somebody adds a new parameter to matrix? The code has to be copy/pasted in every single function that did not want to call __init__?

All right. But then there is something wrong in the docstring of matrix0.Matrix

    def __init__(...):
        ...
        Subclasses of ``Matrix`` can safely skip calling
        ``Matrix.__init__`` provided they take care of initializing these
        attributes themselves.
        ...
  • Why?

All this was just old Cython code at the time Cython was not able to understand l[i] when l is a cdef list.


archive/issue_comments_249738.json:

{
    "body": "<div id=\"comment:5\"></div>\n\nBranch pushed to git repo; I updated commit sha1. New commits:\n<table><tr><td><a href=\"https://github.com/sagemath/sagetrac-mirror/commit/88525b82a790bb4903ab19757acc7a3acd01da72\"><code>88525b8</code></a></td><td><code>Trac 18231: fix documentation</code></td></tr></table>\n",
    "created_at": "2015-04-19T10:47:02Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/18231",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/18231#issuecomment-249738",
    "user": "https://github.com/sagetrac-git"
}

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

88525b8Trac 18231: fix documentation

archive/issue_comments_249739.json:

{
    "body": "Changed commit from **[`da310db`](https://github.com/sagemath/sagetrac-mirror/commit/da310db9b3b4238121b1a1ad2c3d5e36a5e846b5)** to **[`88525b8`](https://github.com/sagemath/sagetrac-mirror/commit/88525b82a790bb4903ab19757acc7a3acd01da72)**",
    "created_at": "2015-04-19T10:47:02Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/18231",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/18231#issuecomment-249739",
    "user": "https://github.com/sagetrac-git"
}

Changed commit from da310db to 88525b8


archive/issue_comments_249740.json:

{
    "body": "<div id=\"comment:6\"></div>\n\nBranch pushed to git repo; I updated commit sha1. New commits:\n<table><tr><td><a href=\"https://github.com/sagemath/sagetrac-mirror/commit/715f49ecab8ae8bb66725be77da51ce6eb8f8738\"><code>715f49e</code></a></td><td><code>Trac 18231: fix doctests</code></td></tr></table>\n",
    "created_at": "2015-04-19T12:53:21Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/18231",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/18231#issuecomment-249740",
    "user": "https://github.com/sagetrac-git"
}

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

715f49eTrac 18231: fix doctests

archive/issue_comments_249741.json:

{
    "body": "Changed commit from **[`88525b8`](https://github.com/sagemath/sagetrac-mirror/commit/88525b82a790bb4903ab19757acc7a3acd01da72)** to **[`715f49e`](https://github.com/sagemath/sagetrac-mirror/commit/715f49ecab8ae8bb66725be77da51ce6eb8f8738)**",
    "created_at": "2015-04-19T12:53:21Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/18231",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/18231#issuecomment-249741",
    "user": "https://github.com/sagetrac-git"
}

Changed commit from 88525b8 to 715f49e


archive/issue_comments_249742.json:

{
    "body": "<div id=\"comment:7\" align=\"right\">comment:7</div>\n\nYoooooo !\n\n> I made a big effort to separate the commit at least! It is now in the description of the ticket.\n\nThanks\n\n> No choice. This is a cdef function defined in `structure.element.Element` and you can not change the signature in children classes. I thought it was more readable this way.\n\nOh, okay!\n\n> All right. But then there is something wrong in the docstring of `matrix0.Matrix`\n\nHmmmm.... Can you at least observe the difference? All that this `__init__` function does is precisely what you do. Can you measure this function call when your function copies lists around? `O_o`\n\n> All this was just old Cython code at the time Cython was not able to understand `l[i]` when `l` is a `cdef list`.\n\nOkay!\n\nNathann",
    "created_at": "2015-04-19T19:15:22Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/18231",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/18231#issuecomment-249742",
    "user": "https://github.com/nathanncohen"
}
comment:7

Yoooooo !

I made a big effort to separate the commit at least! It is now in the description of the ticket.

Thanks

No choice. This is a cdef function defined in structure.element.Element and you can not change the signature in children classes. I thought it was more readable this way.

Oh, okay!

All right. But then there is something wrong in the docstring of matrix0.Matrix

Hmmmm.... Can you at least observe the difference? All that this __init__ function does is precisely what you do. Can you measure this function call when your function copies lists around? O_o

All this was just old Cython code at the time Cython was not able to understand l[i] when l is a cdef list.

Okay!

Nathann


archive/issue_comments_249743.json:

{
    "body": "<div id=\"comment:8\" align=\"right\">comment:8</div>\n\nOkay, I give up. Sorry, I just don't know these classes enough, I barely ever used them, I would do a terrible job as a reviewer.\n\nNathann",
    "created_at": "2015-04-19T19:19:51Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/18231",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/18231#issuecomment-249743",
    "user": "https://github.com/nathanncohen"
}
comment:8

Okay, I give up. Sorry, I just don't know these classes enough, I barely ever used them, I would do a terrible job as a reviewer.

Nathann


archive/issue_comments_249744.json:

{
    "body": "Description changed:\n``````diff\n--- \n+++ \n@@ -40,3 +40,5 @@\n sage: M._matrix_class([1,5,3,-1,2,3], coerce=False, copy=False)\n ```\n - we simplify the main element constructor of matrix, namely `MatrixSpace.matrix` and gain a significant speedup.\n+\n+follow up: #18258\n``````\n",
    "created_at": "2015-04-20T09:40:24Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/18231",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/18231#issuecomment-249744",
    "user": "https://github.com/videlec"
}

Description changed:

--- 
+++ 
@@ -40,3 +40,5 @@
 sage: M._matrix_class([1,5,3,-1,2,3], coerce=False, copy=False)
 ```
 - we simplify the main element constructor of matrix, namely `MatrixSpace.matrix` and gain a significant speedup.
+
+follow up: #18258

archive/issue_comments_249745.json:

{
    "body": "<div id=\"comment:10\" align=\"right\">comment:10</div>\n\nneeds rebase, does not apply",
    "created_at": "2015-08-09T18:43:14Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/18231",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/18231#issuecomment-249745",
    "user": "https://github.com/fchapoton"
}
comment:10

needs rebase, does not apply


archive/issue_events_256852.json:

{
    "actor": "https://github.com/fchapoton",
    "created_at": "2015-08-09T18:43:14Z",
    "event": "unlabeled",
    "issue": "https://github.com/sagemath/sage/issues/18231",
    "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/18231#event-256852"
}

archive/issue_events_256853.json:

{
    "actor": "https://github.com/fchapoton",
    "created_at": "2015-08-09T18:43:14Z",
    "event": "labeled",
    "issue": "https://github.com/sagemath/sage/issues/18231",
    "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/18231#event-256853"
}

archive/issue_comments_249746.json:

{
    "body": "Changed commit from **[`715f49e`](https://github.com/sagemath/sagetrac-mirror/commit/715f49ecab8ae8bb66725be77da51ce6eb8f8738)** to **[`d4fc6cd`](https://github.com/sagemath/sagetrac-mirror/commit/d4fc6cd87dc244113d7662b95a3d1c4f10258a36)**",
    "created_at": "2015-08-13T13:54:24Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/18231",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/18231#issuecomment-249746",
    "user": "https://github.com/sagetrac-git"
}

Changed commit from 715f49e to d4fc6cd


archive/issue_comments_249747.json:

{
    "body": "<div id=\"comment:11\"></div>\n\nBranch pushed to git repo; I updated commit sha1. This was a forced push. Last 10 new commits:\n<table><tr><td><a href=\"https://github.com/sagemath/sagetrac-mirror/commit/2d5549792577c21880eb790218a51daf7633521d\"><code>2d55497</code></a></td><td><code>Trac 18231: constructor + __matrix_class -> _matrix_class</code></td></tr><tr><td><a href=\"https://github.com/sagemath/sagetrac-mirror/commit/5ac117fe6dd2c3407e8e930d360e215a6bff13ac\"><code>5ac117f</code></a></td><td><code>Trac 18231: removed unused import</code></td></tr><tr><td><a href=\"https://github.com/sagemath/sagetrac-mirror/commit/fcd06fab61a61a1d76d98df0aeed7144f15d8410\"><code>fcd06fa</code></a></td><td><code>Trac 18231: get rid of a one year old deprecation</code></td></tr><tr><td><a href=\"https://github.com/sagemath/sagetrac-mirror/commit/b7bf1aa15f810c90ff12a124737b5b07224d2467\"><code>b7bf1aa</code></a></td><td><code>Trac 18231: remove a useless function</code></td></tr><tr><td><a href=\"https://github.com/sagemath/sagetrac-mirror/commit/12c9d0e1918f169fbd1fdacf1200a25ff1ce3e26\"><code>12c9d0e</code></a></td><td><code>Trac 18231: set .__call__() as an alias of .matrix()</code></td></tr><tr><td><a href=\"https://github.com/sagemath/sagetrac-mirror/commit/d59a9db1533339ed97b9692bda87e4ee71e5e576\"><code>d59a9db</code></a></td><td><code>Trac 18231: rewrite .matrix()</code></td></tr><tr><td><a href=\"https://github.com/sagemath/sagetrac-mirror/commit/46d8b6b098980c59d15612fc8d078c97fdf2f699\"><code>46d8b6b</code></a></td><td><code>Trac 18231: fix dict_to_list</code></td></tr><tr><td><a href=\"https://github.com/sagemath/sagetrac-mirror/commit/aa7ddb2ce1cac41142e5a45bdea3bcc322d4956a\"><code>aa7ddb2</code></a></td><td><code>Trac 18231: typo</code></td></tr><tr><td><a href=\"https://github.com/sagemath/sagetrac-mirror/commit/03fe3b1a36aad650f3a1f0fb751533efaf311720\"><code>03fe3b1</code></a></td><td><code>Trac 18231: fix documentation</code></td></tr><tr><td><a href=\"https://github.com/sagemath/sagetrac-mirror/commit/d4fc6cd87dc244113d7662b95a3d1c4f10258a36\"><code>d4fc6cd</code></a></td><td><code>Trac 18231: fix doctests</code></td></tr></table>\n",
    "created_at": "2015-08-13T13:54:24Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/18231",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/18231#issuecomment-249747",
    "user": "https://github.com/sagetrac-git"
}

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

2d55497Trac 18231: constructor + __matrix_class -> _matrix_class
5ac117fTrac 18231: removed unused import
fcd06faTrac 18231: get rid of a one year old deprecation
b7bf1aaTrac 18231: remove a useless function
12c9d0eTrac 18231: set .__call__() as an alias of .matrix()
d59a9dbTrac 18231: rewrite .matrix()
46d8b6bTrac 18231: fix dict_to_list
aa7ddb2Trac 18231: typo
03fe3b1Trac 18231: fix documentation
d4fc6cdTrac 18231: fix doctests

archive/issue_events_256854.json:

{
    "actor": "https://github.com/videlec",
    "created_at": "2015-08-13T13:55:37Z",
    "event": "unlabeled",
    "issue": "https://github.com/sagemath/sage/issues/18231",
    "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/18231#event-256854"
}

archive/issue_events_256855.json:

{
    "actor": "https://github.com/videlec",
    "created_at": "2015-08-13T13:55:37Z",
    "event": "labeled",
    "issue": "https://github.com/sagemath/sage/issues/18231",
    "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/18231#event-256855"
}

archive/issue_comments_249748.json:

{
    "body": "<div id=\"comment:12\" align=\"right\">comment:12</div>\n\nWhether to see if patchbot is happy.",
    "created_at": "2015-08-13T13:55:37Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/18231",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/18231#issuecomment-249748",
    "user": "https://github.com/videlec"
}
comment:12

Whether to see if patchbot is happy.


archive/issue_events_256856.json:

{
    "actor": "https://github.com/videlec",
    "created_at": "2015-08-13T15:01:15Z",
    "event": "unlabeled",
    "issue": "https://github.com/sagemath/sage/issues/18231",
    "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/18231#event-256856"
}

archive/issue_events_256857.json:

{
    "actor": "https://github.com/videlec",
    "created_at": "2015-08-13T15:01:15Z",
    "event": "labeled",
    "issue": "https://github.com/sagemath/sage/issues/18231",
    "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/18231#event-256857"
}

archive/issue_comments_249749.json:

{
    "body": "Dependencies: **#18231**",
    "created_at": "2015-08-13T15:01:15Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/18231",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/18231#issuecomment-249749",
    "user": "https://github.com/videlec"
}

Dependencies: #18231


archive/issue_comments_249750.json:

{
    "body": "Changed dependencies from **#18231** to **#19026**",
    "created_at": "2015-08-13T15:01:29Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/18231",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/18231#issuecomment-249750",
    "user": "https://github.com/videlec"
}

Changed dependencies from #18231 to #19026


archive/issue_comments_249751.json:

{
    "body": "<div id=\"comment:15\"></div>\n\nBranch pushed to git repo; I updated commit sha1. This was a forced push. Last 10 new commits:\n<table><tr><td><a href=\"https://github.com/sagemath/sagetrac-mirror/commit/268cfc3547da9d34d41bdbe46e8344849b762fc0\"><code>268cfc3</code></a></td><td><code>Trac 18231: move _get_matrix_class out of the class</code></td></tr><tr><td><a href=\"https://github.com/sagemath/sagetrac-mirror/commit/66e50682205e719721780791af479eb402654fb8\"><code>66e5068</code></a></td><td><code>Trac 18231: constructor + __matrix_class -> _matrix_class</code></td></tr><tr><td><a href=\"https://github.com/sagemath/sagetrac-mirror/commit/c9a82f24cfafa64d239f178c019c494613a01349\"><code>c9a82f2</code></a></td><td><code>Trac 18231: removed unused import</code></td></tr><tr><td><a href=\"https://github.com/sagemath/sagetrac-mirror/commit/321d0730314a3756bdfc5422772783829b35aef0\"><code>321d073</code></a></td><td><code>Trac 18231: get rid of a one year old deprecation</code></td></tr><tr><td><a href=\"https://github.com/sagemath/sagetrac-mirror/commit/6111c37e89bdb7722a2b923fef5f4e96fe953594\"><code>6111c37</code></a></td><td><code>Trac 18231: remove a useless function</code></td></tr><tr><td><a href=\"https://github.com/sagemath/sagetrac-mirror/commit/cb6b82d13d773eef5c508059732dad3ddd1d5af7\"><code>cb6b82d</code></a></td><td><code>Trac 18231: set .__call__() as an alias of .matrix()</code></td></tr><tr><td><a href=\"https://github.com/sagemath/sagetrac-mirror/commit/2dd0d098312ee4c241fabe732e7834b400f518b6\"><code>2dd0d09</code></a></td><td><code>Trac 18231: rewrite .matrix()</code></td></tr><tr><td><a href=\"https://github.com/sagemath/sagetrac-mirror/commit/00dd4ac1a204371595be992155bd5ef3b7794400\"><code>00dd4ac</code></a></td><td><code>Trac 18231: fix dict_to_list</code></td></tr><tr><td><a href=\"https://github.com/sagemath/sagetrac-mirror/commit/07ba6810af0e02b35ee756c229d366ecc4492043\"><code>07ba681</code></a></td><td><code>Trac 18231: fix documentation</code></td></tr><tr><td><a href=\"https://github.com/sagemath/sagetrac-mirror/commit/742de47c66db9df0eee3f6200a0b8d2764ab743e\"><code>742de47</code></a></td><td><code>Trac 18231: fix doctests</code></td></tr></table>\n",
    "created_at": "2015-08-13T15:06:06Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/18231",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/18231#issuecomment-249751",
    "user": "https://github.com/sagetrac-git"
}

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

268cfc3Trac 18231: move _get_matrix_class out of the class
66e5068Trac 18231: constructor + __matrix_class -> _matrix_class
c9a82f2Trac 18231: removed unused import
321d073Trac 18231: get rid of a one year old deprecation
6111c37Trac 18231: remove a useless function
cb6b82dTrac 18231: set .__call__() as an alias of .matrix()
2dd0d09Trac 18231: rewrite .matrix()
00dd4acTrac 18231: fix dict_to_list
07ba681Trac 18231: fix documentation
742de47Trac 18231: fix doctests

archive/issue_comments_249752.json:

{
    "body": "Changed commit from **[`d4fc6cd`](https://github.com/sagemath/sagetrac-mirror/commit/d4fc6cd87dc244113d7662b95a3d1c4f10258a36)** to **[`742de47`](https://github.com/sagemath/sagetrac-mirror/commit/742de47c66db9df0eee3f6200a0b8d2764ab743e)**",
    "created_at": "2015-08-13T15:06:06Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/18231",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/18231#issuecomment-249752",
    "user": "https://github.com/sagetrac-git"
}

Changed commit from d4fc6cd to 742de47


archive/issue_events_256858.json:

{
    "actor": "https://github.com/videlec",
    "created_at": "2015-08-13T15:06:41Z",
    "event": "unlabeled",
    "issue": "https://github.com/sagemath/sage/issues/18231",
    "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/18231#event-256858"
}

archive/issue_events_256859.json:

{
    "actor": "https://github.com/videlec",
    "created_at": "2015-08-13T15:06:41Z",
    "event": "labeled",
    "issue": "https://github.com/sagemath/sage/issues/18231",
    "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/18231#event-256859"
}

archive/issue_comments_249753.json:

{
    "body": "<div id=\"comment:16\" align=\"right\">comment:16</div>\n\nI moved the first commit to another ticket (#19026).",
    "created_at": "2015-08-13T15:06:41Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/18231",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/18231#issuecomment-249753",
    "user": "https://github.com/videlec"
}
comment:16

I moved the first commit to another ticket (#19026).


archive/issue_comments_249754.json:

{
    "body": "<div id=\"comment:17\" align=\"right\">comment:17</div>\n\nneeds rebase",
    "created_at": "2015-09-11T06:44:11Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/18231",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/18231#issuecomment-249754",
    "user": "https://github.com/fchapoton"
}
comment:17

needs rebase


archive/issue_events_256860.json:

{
    "actor": "https://github.com/fchapoton",
    "created_at": "2015-09-11T06:44:11Z",
    "event": "unlabeled",
    "issue": "https://github.com/sagemath/sage/issues/18231",
    "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/18231#event-256860"
}

archive/issue_events_256861.json:

{
    "actor": "https://github.com/fchapoton",
    "created_at": "2015-09-11T06:44:11Z",
    "event": "labeled",
    "issue": "https://github.com/sagemath/sage/issues/18231",
    "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/18231#event-256861"
}

archive/issue_comments_249755.json:

{
    "body": "Description changed:\n``````diff\n--- \n+++ \n@@ -28,7 +28,7 @@\n 100 loops, best of 3: 6.49 ms per loop\n ```\n \n-This is one of the reason why the creation of Polyhedron is so slow (see #18213).\n+This is one of the reasons why the creation of Polyhedron is so slow (see #18213). (This has now improved since the creation of the ticket)\n \n In the branch we:\n - replace some old Cython code in `matrix_generic_dense` and `matrix0` and do some optimization\n``````\n",
    "created_at": "2017-03-10T11:31:28Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/18231",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/18231#issuecomment-249755",
    "user": "https://github.com/jplab"
}

Description changed:

--- 
+++ 
@@ -28,7 +28,7 @@
 100 loops, best of 3: 6.49 ms per loop
 ```
 
-This is one of the reason why the creation of Polyhedron is so slow (see #18213).
+This is one of the reasons why the creation of Polyhedron is so slow (see #18213). (This has now improved since the creation of the ticket)
 
 In the branch we:
 - replace some old Cython code in `matrix_generic_dense` and `matrix0` and do some optimization

archive/issue_comments_249756.json:

{
    "body": "Changed author from **Vincent Delecroix** to none",
    "created_at": "2022-02-19T18:22:28Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/18231",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/18231#issuecomment-249756",
    "user": "https://github.com/mkoeppe"
}

Changed author from Vincent Delecroix to none


archive/issue_events_256862.json:

{
    "actor": "https://github.com/mkoeppe",
    "created_at": "2022-02-19T18:22:28Z",
    "event": "unlabeled",
    "issue": "https://github.com/sagemath/sage/issues/18231",
    "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/18231#event-256862"
}

archive/issue_events_256863.json:

{
    "actor": "https://github.com/mkoeppe",
    "created_at": "2022-02-19T18:22:28Z",
    "event": "labeled",
    "issue": "https://github.com/sagemath/sage/issues/18231",
    "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/18231#event-256863"
}

archive/issue_comments_249757.json:

{
    "body": "<div id=\"comment:19\" align=\"right\">comment:19</div>\n\nOutdated, this was fixed in #24742.\n\n```\nsage: %timeit test1()\n3.91 ms \u00b1 47.3 \u00b5s per loop (mean \u00b1 std. dev. of 7 runs, 100 loops each)\nsage: %timeit test2()\n3.03 ms \u00b1 19.4 \u00b5s per loop (mean \u00b1 std. dev. of 7 runs, 100 loops each)\n```",
    "created_at": "2022-02-19T18:22:28Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/18231",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/18231#issuecomment-249757",
    "user": "https://github.com/mkoeppe"
}
comment:19

Outdated, this was fixed in #24742.

sage: %timeit test1()
3.91 ms ± 47.3 µs per loop (mean ± std. dev. of 7 runs, 100 loops each)
sage: %timeit test2()
3.03 ms ± 19.4 µs per loop (mean ± std. dev. of 7 runs, 100 loops each)

archive/issue_events_256864.json:

{
    "actor": "https://github.com/mkoeppe",
    "created_at": "2022-02-19T18:22:28Z",
    "event": "demilestoned",
    "issue": "https://github.com/sagemath/sage/issues/18231",
    "milestone_number": null,
    "milestone_title": "sage-6.7",
    "type": "issue_event",
    "url": "https://github.com/sagemath/sage/issues/18231#event-256864"
}

archive/issue_comments_249758.json:

{
    "body": "Changed dependencies from **#19026** to none",
    "created_at": "2022-02-19T18:51:44Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/18231",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/18231#issuecomment-249758",
    "user": "https://github.com/videlec"
}

Changed dependencies from #19026 to none


archive/issue_comments_249759.json:

{
    "body": "Changed branch from **[u/vdelecroix/18231](https://github.com/sagemath/sagetrac-mirror/tree/u/vdelecroix/18231)** to none",
    "created_at": "2022-02-19T18:51:44Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/18231",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/18231#issuecomment-249759",
    "user": "https://github.com/videlec"
}

Changed branch from u/vdelecroix/18231 to none


archive/issue_events_256865.json:

{
    "actor": "https://github.com/videlec",
    "created_at": "2022-02-19T18:51:44Z",
    "event": "unlabeled",
    "issue": "https://github.com/sagemath/sage/issues/18231",
    "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/18231#event-256865"
}

archive/issue_events_256866.json:

{
    "actor": "https://github.com/videlec",
    "created_at": "2022-02-19T18:51:44Z",
    "event": "labeled",
    "issue": "https://github.com/sagemath/sage/issues/18231",
    "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/18231#event-256866"
}

archive/issue_comments_249760.json:

{
    "body": "Changed commit from **[`742de47`](https://github.com/sagemath/sagetrac-mirror/commit/742de47c66db9df0eee3f6200a0b8d2764ab743e)** to none",
    "created_at": "2022-02-19T18:51:44Z",
    "formatter": "markdown",
    "issue": "https://github.com/sagemath/sage/issues/18231",
    "type": "issue_comment",
    "url": "https://github.com/sagemath/sage/issues/18231#issuecomment-249760",
    "user": "https://github.com/videlec"
}

Changed commit from 742de47 to none


archive/issue_events_256867.json:

{
    "actor": "https://github.com/fchapoton",
    "created_at": "2022-02-25T10:34:15Z",
    "event": "unlabeled",
    "issue": "https://github.com/sagemath/sage/issues/18231",
    "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/18231#event-256867"
}

archive/issue_events_256868.json:

{
    "actor": "https://github.com/fchapoton",
    "created_at": "2022-02-25T10:34:15Z",
    "event": "closed",
    "issue": "https://github.com/sagemath/sage/issues/18231",
    "type": "issue_event",
    "url": "https://github.com/sagemath/sage/issues/18231#event-256868"
}

archive/issue_events_256869.json:

{
    "actor": "https://github.com/fchapoton",
    "created_at": "2022-02-25T10:34:15Z",
    "event": "unlabeled",
    "issue": "https://github.com/sagemath/sage/issues/18231",
    "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/18231#event-256869"
}

archive/issue_events_256870.json:

{
    "actor": "https://github.com/fchapoton",
    "created_at": "2022-02-25T10:34:15Z",
    "event": "labeled",
    "issue": "https://github.com/sagemath/sage/issues/18231",
    "label": "https://github.com/sagemath/sage/labels/duplicate",
    "label_color": "c6c6c6",
    "label_name": "duplicate",
    "label_text_color": "ffffff",
    "type": "issue_event",
    "url": "https://github.com/sagemath/sage/issues/18231#event-256870"
}