archive/issues_020449.json:
{
"assignees": [],
"body": "<div id=\"comment:0\"></div>\n\nFor parents and elements implemented in Cython, category lookup is emulated in `__getattr__` using `getattr_from_other_class`. In this ticket, this is refactored a bit:\n\n1. We should not pick up special attributes from `type`, there is no point in returning the type name of the category here (similarly for `__dict__`, `__mro__`, ...):\n\n```\nsage: ZZ(1).__name__\n'JoinCategory.element_class'\nsage: ZZ.__name__\n'JoinCategory.parent_class'\n```\nThis change causes a few failures which need to be fixed.\n\n2. The implementation of `getattr_from_other_class` is not very robust. For example, static methods are not supported. We fix this by using low-level Python functions to get the attribute and we manually call the descriptor `__get__` if needed.\n\n3. We shouldn't do anything special with double-underscore private attributes: in plain Python, this is implemented by the parser and not by `getattr()`. So `__getattr__` would already receive the mangled private name.\n\n4. Some of the changes broke `_sage_src_lines_`, so we also change that: currently, `_sage_src_lines_()` is used both to get the source lines of a class (e.g. dynamic classes) and an instance (e.g. cached functions). We change this to always mean the source lines of an instance, which makes things clearer.\n\n5. The lookup using `getattr_from_other_class` is about 9 times slower than a normal method lookup::\n\n```\nsage: def foo(self): return\nsage: Sets().element_class.foo = foo\nsage: def g(x):\n....: for i in range(1000):\n....: x.foo()\n\nsage: x = Semigroups().example().an_element()\nsage: y = 1\n\nsage: %timeit g(x)\n10000 loops, best of 3: 115 \u00b5s per loop\n\nsage: %timeit g(y)\n1000 loops, best of 3: 1.18 ms per loop\n```\nWe improve on this by roughly a factor 2 (but even then, it's still a lot slower than usual method lookup).\n\nNOTE: This needs a trivial Cython patch, see #21030 for the Cython upgrade.\n\nDepends on #21030\nDepends on #21409\n\nUpstream: **Fixed upstream, in a later stable release.**\n\nCC: @jdemeyer @tscrim\n\nComponent: **categories**\n\nAuthor: **Jeroen Demeyer**\n\nBranch/Commit: **[`74041f3`](https://github.com/sagemath/sagetrac-mirror/commit/74041f30d8de4c15055345f14340a4f443236ffa)**\n\nReviewer: **Vincent Delecroix**\n\n_Issue created by migration from https://trac.sagemath.org/ticket/20686_\n\n",
"closed_at": "2016-09-04T13:38:02Z",
"created_at": "2016-05-26T13:36:13Z",
"labels": [
"https://github.com/sagemath/sage/labels/c%3A%20categories",
"https://github.com/sagemath/sage/labels/p%3A%20major%20/%203",
"https://github.com/sagemath/sage/labels/enhancement"
],
"milestone": "https://github.com/sagemath/sage/milestones/sage-7.4",
"reactions": [],
"repository": "https://github.com/sagemath/sage",
"title": "Refactor getattr_from_other_class() for lookup of methods in categories",
"type": "issue",
"updated_at": "2016-09-04T13:38:02Z",
"url": "https://github.com/sagemath/sage/issues/20686",
"user": "https://github.com/nthiery"
}For parents and elements implemented in Cython, category lookup is emulated in __getattr__ using getattr_from_other_class. In this ticket, this is refactored a bit:
- We should not pick up special attributes from
type, there is no point in returning the type name of the category here (similarly for__dict__,__mro__, ...):
sage: ZZ(1).__name__
'JoinCategory.element_class'
sage: ZZ.__name__
'JoinCategory.parent_class'
This change causes a few failures which need to be fixed.
-
The implementation of
getattr_from_other_classis not very robust. For example, static methods are not supported. We fix this by using low-level Python functions to get the attribute and we manually call the descriptor__get__if needed. -
We shouldn't do anything special with double-underscore private attributes: in plain Python, this is implemented by the parser and not by
getattr(). So__getattr__would already receive the mangled private name. -
Some of the changes broke
_sage_src_lines_, so we also change that: currently,_sage_src_lines_()is used both to get the source lines of a class (e.g. dynamic classes) and an instance (e.g. cached functions). We change this to always mean the source lines of an instance, which makes things clearer. -
The lookup using
getattr_from_other_classis about 9 times slower than a normal method lookup::
sage: def foo(self): return
sage: Sets().element_class.foo = foo
sage: def g(x):
....: for i in range(1000):
....: x.foo()
sage: x = Semigroups().example().an_element()
sage: y = 1
sage: %timeit g(x)
10000 loops, best of 3: 115 µs per loop
sage: %timeit g(y)
1000 loops, best of 3: 1.18 ms per loop
We improve on this by roughly a factor 2 (but even then, it's still a lot slower than usual method lookup).
NOTE: This needs a trivial Cython patch, see #21030 for the Cython upgrade.
Depends on #21030 Depends on #21409
Upstream: Fixed upstream, in a later stable release.
CC: @jdemeyer @tscrim
Component: categories
Author: Jeroen Demeyer
Branch/Commit: 74041f3
Reviewer: Vincent Delecroix
Issue created by migration from https://trac.sagemath.org/ticket/20686
archive/issue_events_288651.json:
{
"actor": "https://github.com/nthiery",
"created_at": "2016-05-26T13:36:13Z",
"event": "milestoned",
"issue": "https://github.com/sagemath/sage/issues/20686",
"milestone_number": null,
"milestone_title": "sage-7.3",
"type": "issue_event",
"url": "https://github.com/sagemath/sage/issues/20686#event-288651"
}archive/issue_events_288652.json:
{
"actor": "https://github.com/nthiery",
"created_at": "2016-05-26T13:36:13Z",
"event": "labeled",
"issue": "https://github.com/sagemath/sage/issues/20686",
"label": "https://github.com/sagemath/sage/labels/c%3A%20categories",
"label_color": "0000ff",
"label_name": "c: categories",
"label_text_color": "ffffff",
"type": "issue_event",
"url": "https://github.com/sagemath/sage/issues/20686#event-288652"
}archive/issue_events_288653.json:
{
"actor": "https://github.com/nthiery",
"created_at": "2016-05-26T13:36:13Z",
"event": "labeled",
"issue": "https://github.com/sagemath/sage/issues/20686",
"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/20686#event-288653"
}archive/issue_events_288654.json:
{
"actor": "https://github.com/nthiery",
"created_at": "2016-05-26T13:36:13Z",
"event": "labeled",
"issue": "https://github.com/sagemath/sage/issues/20686",
"label": "https://github.com/sagemath/sage/labels/enhancement",
"label_color": "696969",
"label_name": "enhancement",
"label_text_color": "ffffff",
"type": "issue_event",
"url": "https://github.com/sagemath/sage/issues/20686#event-288654"
}archive/issue_comments_299085.json:
{
"body": "<div id=\"comment:2\" align=\"right\">comment:2</div>\n\nI might have some ideas... discuss in Meudon?",
"created_at": "2016-05-26T14:56:35Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20686",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20686#issuecomment-299085",
"user": "https://github.com/jdemeyer"
}comment:2
I might have some ideas... discuss in Meudon?
archive/issue_comments_299086.json:
{
"body": "<div id=\"comment:3\" align=\"right\">comment:3</div>\n\n+1",
"created_at": "2016-05-26T15:18:30Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20686",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20686#issuecomment-299086",
"user": "https://github.com/nthiery"
}comment:3
+1
archive/issue_comments_299087.json:
{
"body": "Description changed:\n``````diff\n--- \n+++ \n@@ -23,5 +23,9 @@\n So there should be room for optimization (or using a different\n approach for emulating this inheritance).\n \n-First step: profiling ...\n+Besides this, we should not pick up special attributes from `type`, there is no point in returning `type.__name__` here:\n \n+```\n+sage: getattr_from_other_class(1, type, \"__name__\")\n+'type'\n+```\n``````\n",
"created_at": "2016-06-14T13:44:33Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20686",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20686#issuecomment-299087",
"user": "https://github.com/jdemeyer"
}Description changed:
---
+++
@@ -23,5 +23,9 @@
So there should be room for optimization (or using a different
approach for emulating this inheritance).
-First step: profiling ...
+Besides this, we should not pick up special attributes from `type`, there is no point in returning `type.__name__` here:
+```
+sage: getattr_from_other_class(1, type, "__name__")
+'type'
+```archive/issue_comments_299088.json:
{
"body": "Author: **Jeroen Demeyer**",
"created_at": "2016-06-14T13:44:33Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20686",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20686#issuecomment-299088",
"user": "https://github.com/jdemeyer"
}Author: Jeroen Demeyer
archive/issue_comments_299089.json:
{
"body": "Branch: **[u/jdemeyer/optimize_method_lookup_from_the_categories_for_instances_of_cython_classes](https://github.com/sagemath/sagetrac-mirror/tree/u/jdemeyer/optimize_method_lookup_from_the_categories_for_instances_of_cython_classes)**",
"created_at": "2016-06-14T13:46:29Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20686",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20686#issuecomment-299089",
"user": "https://github.com/jdemeyer"
}Branch: u/jdemeyer/optimize_method_lookup_from_the_categories_for_instances_of_cython_classes
archive/issue_comments_299090.json:
{
"body": "Dependencies: **#20825**",
"created_at": "2016-06-14T15:13:30Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20686",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20686#issuecomment-299090",
"user": "https://github.com/jdemeyer"
}Dependencies: #20825
archive/issue_comments_299091.json:
{
"body": "<div id=\"comment:6\"></div>\n\nNew commits:\n<table><tr><td><a href=\"https://github.com/sagemath/sagetrac-mirror/commit/a143a32a7812ec2405d73487050c425816d74794\"><code>a143a32</code></a></td><td><code>EvaluationMethods should be a new-style class</code></td></tr><tr><td><a href=\"https://github.com/sagemath/sagetrac-mirror/commit/bbd0b05c11b0431f02b34c1b55bed6b775de7fa8\"><code>bbd0b05</code></a></td><td><code>Improve getattr_from_other_class</code></td></tr></table>\n",
"created_at": "2016-06-14T15:13:30Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20686",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20686#issuecomment-299091",
"user": "https://github.com/jdemeyer"
}New commits:
a143a32 | EvaluationMethods should be a new-style class |
bbd0b05 | Improve getattr_from_other_class |
archive/issue_comments_299092.json:
{
"body": "Commit: **[`bbd0b05`](https://github.com/sagemath/sagetrac-mirror/commit/bbd0b05c11b0431f02b34c1b55bed6b775de7fa8)**",
"created_at": "2016-06-14T15:13:30Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20686",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20686#issuecomment-299092",
"user": "https://github.com/jdemeyer"
}Commit: bbd0b05
archive/issue_comments_299093.json:
{
"body": "Changed commit from **[`bbd0b05`](https://github.com/sagemath/sagetrac-mirror/commit/bbd0b05c11b0431f02b34c1b55bed6b775de7fa8)** to **[`3cd449f`](https://github.com/sagemath/sagetrac-mirror/commit/3cd449f14dc1b8c07d326ae1192067f4fdf7e644)**",
"created_at": "2016-06-14T15:58:40Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20686",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20686#issuecomment-299093",
"user": "https://github.com/sagetrac-git"
}Changed commit from bbd0b05 to 3cd449f
archive/issue_comments_299094.json:
{
"body": "<div id=\"comment:7\"></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/44e80b30d32d5b6d35c00edecf81d4313dc1c11f\"><code>44e80b3</code></a></td><td><code>EvaluationMethods should be a new-style class</code></td></tr><tr><td><a href=\"https://github.com/sagemath/sagetrac-mirror/commit/3cd449f14dc1b8c07d326ae1192067f4fdf7e644\"><code>3cd449f</code></a></td><td><code>Improve getattr_from_other_class</code></td></tr></table>\n",
"created_at": "2016-06-14T15:58:40Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20686",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20686#issuecomment-299094",
"user": "https://github.com/sagetrac-git"
}Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
44e80b3 | EvaluationMethods should be a new-style class |
3cd449f | Improve getattr_from_other_class |
archive/issue_comments_299095.json:
{
"body": "Upstream: **Reported upstream. No feedback yet.**",
"created_at": "2016-06-14T16:07:32Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20686",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20686#issuecomment-299095",
"user": "https://github.com/jdemeyer"
}Upstream: Reported upstream. No feedback yet.
archive/issue_comments_299096.json:
{
"body": "Description changed:\n``````diff\n--- \n+++ \n@@ -29,3 +29,5 @@\n sage: getattr_from_other_class(1, type, \"__name__\")\n 'type'\n ```\n+\n+This needs a trivial Cython patch: https://github.com/cython/cython/pull/522\n``````\n",
"created_at": "2016-06-14T16:07:32Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20686",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20686#issuecomment-299096",
"user": "https://github.com/jdemeyer"
}Description changed:
---
+++
@@ -29,3 +29,5 @@
sage: getattr_from_other_class(1, type, "__name__")
'type'
```
+
+This needs a trivial Cython patch: https://github.com/cython/cython/pull/522archive/issue_comments_299097.json:
{
"body": "Description changed:\n``````diff\n--- \n+++ \n@@ -6,17 +6,17 @@\n ```\n sage: def foo(self): return\n sage: Sets().element_class.foo = foo\n-\n sage: def g(x):\n ....: for i in range(1000):\n ....: x.foo()\n \n sage: x = Semigroups().example().an_element()\n+sage: y = 1\n+\n sage: %timeit g(x)\n 10000 loops, best of 3: 115 \u00b5s per loop\n \n-sage: x = 1\n-sage: %timeit g(x)\n+sage: %timeit g(y)\n 1000 loops, best of 3: 1.18 ms per loop\n ```\n \n``````\n",
"created_at": "2016-06-14T16:29:46Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20686",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20686#issuecomment-299097",
"user": "https://github.com/jdemeyer"
}Description changed:
---
+++
@@ -6,17 +6,17 @@
```
sage: def foo(self): return
sage: Sets().element_class.foo = foo
-
sage: def g(x):
....: for i in range(1000):
....: x.foo()
sage: x = Semigroups().example().an_element()
+sage: y = 1
+
sage: %timeit g(x)
10000 loops, best of 3: 115 µs per loop
-sage: x = 1
-sage: %timeit g(x)
+sage: %timeit g(y)
1000 loops, best of 3: 1.18 ms per loop
```
archive/issue_comments_299098.json:
{
"body": "Changed upstream from **Reported upstream. No feedback yet.** to **Fixed upstream, but not in a stable release.**",
"created_at": "2016-06-14T19:46:35Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20686",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20686#issuecomment-299098",
"user": "https://github.com/jdemeyer"
}Changed upstream from Reported upstream. No feedback yet. to Fixed upstream, but not in a stable release.
archive/issue_comments_299099.json:
{
"body": "<div id=\"comment:11\"></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/e906b7e6201c92438e41055b08a2ac085f9aa6b7\"><code>e906b7e</code></a></td><td><code>Improve getattr_from_other_class</code></td></tr></table>\n",
"created_at": "2016-06-14T19:56:00Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20686",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20686#issuecomment-299099",
"user": "https://github.com/sagetrac-git"
}Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
e906b7e | Improve getattr_from_other_class |
archive/issue_comments_299100.json:
{
"body": "Changed commit from **[`3cd449f`](https://github.com/sagemath/sagetrac-mirror/commit/3cd449f14dc1b8c07d326ae1192067f4fdf7e644)** to **[`e906b7e`](https://github.com/sagemath/sagetrac-mirror/commit/e906b7e6201c92438e41055b08a2ac085f9aa6b7)**",
"created_at": "2016-06-14T19:56:00Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20686",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20686#issuecomment-299100",
"user": "https://github.com/sagetrac-git"
}Changed commit from 3cd449f to e906b7e
archive/issue_comments_299101.json:
{
"body": "Description changed:\n``````diff\n--- \n+++ \n@@ -23,7 +23,7 @@\n So there should be room for optimization (or using a different\n approach for emulating this inheritance).\n \n-Besides this, we should not pick up special attributes from `type`, there is no point in returning `type.__name__` here:\n+Besides this, we should not pick up special attributes from `type`, there is no point in returning `type.__name__` here (similarly for `__dict__`, `__mro__`, ...):\n \n ```\n sage: getattr_from_other_class(1, type, \"__name__\")\n``````\n",
"created_at": "2016-06-14T20:53:59Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20686",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20686#issuecomment-299101",
"user": "https://github.com/jdemeyer"
}Description changed:
---
+++
@@ -23,7 +23,7 @@
So there should be room for optimization (or using a different
approach for emulating this inheritance).
-Besides this, we should not pick up special attributes from `type`, there is no point in returning `type.__name__` here:
+Besides this, we should not pick up special attributes from `type`, there is no point in returning `type.__name__` here (similarly for `__dict__`, `__mro__`, ...):
```
sage: getattr_from_other_class(1, type, "__name__")archive/issue_comments_299102.json:
{
"body": "Changed commit from **[`e906b7e`](https://github.com/sagemath/sagetrac-mirror/commit/e906b7e6201c92438e41055b08a2ac085f9aa6b7)** to **[`92f4520`](https://github.com/sagemath/sagetrac-mirror/commit/92f4520d01640379590d6c5974622f3d6f396e9b)**",
"created_at": "2016-06-14T20:54:33Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20686",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20686#issuecomment-299102",
"user": "https://github.com/sagetrac-git"
}Changed commit from e906b7e to 92f4520
archive/issue_comments_299103.json:
{
"body": "<div id=\"comment:13\"></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/92f4520d01640379590d6c5974622f3d6f396e9b\"><code>92f4520</code></a></td><td><code>Improve getattr_from_other_class</code></td></tr></table>\n",
"created_at": "2016-06-14T20:54:33Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20686",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20686#issuecomment-299103",
"user": "https://github.com/sagetrac-git"
}Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
92f4520 | Improve getattr_from_other_class |
archive/issue_comments_299104.json:
{
"body": "Description changed:\n``````diff\n--- \n+++ \n@@ -1,7 +1,4 @@\n-For instances of Cython classes, the inheritance from categories is\n-emulated by a manual lookup in\n-`sage.structure.element.Element.__getattr__`. This lookup is about 10\n-times slower than a normal method lookup::\n+For instances of Cython classes, the inheritance from categories is emulated by a manual lookup in `sage.structure.element.Element.__getattr__`. This lookup is about 10 times slower than a normal method lookup::\n \n ```\n sage: def foo(self): return\n@@ -30,4 +27,4 @@\n 'type'\n ```\n \n-This needs a trivial Cython patch: https://github.com/cython/cython/pull/522\n+This needs a trivial Cython patch (accepted by upstream): https://github.com/cython/cython/pull/522\n``````\n",
"created_at": "2016-06-15T07:29:01Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20686",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20686#issuecomment-299104",
"user": "https://github.com/jdemeyer"
}Description changed:
---
+++
@@ -1,7 +1,4 @@
-For instances of Cython classes, the inheritance from categories is
-emulated by a manual lookup in
-`sage.structure.element.Element.__getattr__`. This lookup is about 10
-times slower than a normal method lookup::
+For instances of Cython classes, the inheritance from categories is emulated by a manual lookup in `sage.structure.element.Element.__getattr__`. This lookup is about 10 times slower than a normal method lookup::
```
sage: def foo(self): return
@@ -30,4 +27,4 @@
'type'
```
-This needs a trivial Cython patch: https://github.com/cython/cython/pull/522
+This needs a trivial Cython patch (accepted by upstream): https://github.com/cython/cython/pull/522archive/issue_comments_299105.json:
{
"body": "Description changed:\n``````diff\n--- \n+++ \n@@ -26,5 +26,6 @@\n sage: getattr_from_other_class(1, type, \"__name__\")\n 'type'\n ```\n+This change causes a few failures which need to be fixed.\n \n This needs a trivial Cython patch (accepted by upstream): https://github.com/cython/cython/pull/522\n``````\n",
"created_at": "2016-06-15T09:22:00Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20686",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20686#issuecomment-299105",
"user": "https://github.com/jdemeyer"
}Description changed:
---
+++
@@ -26,5 +26,6 @@
sage: getattr_from_other_class(1, type, "__name__")
'type'
```
+This change causes a few failures which need to be fixed.
This needs a trivial Cython patch (accepted by upstream): https://github.com/cython/cython/pull/522archive/issue_comments_299106.json:
{
"body": "<div id=\"comment:16\"></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/9f529b0c4c8fd3523ebb557c9821e39799c41476\"><code>9f529b0</code></a></td><td><code>Improve getattr_from_other_class</code></td></tr></table>\n",
"created_at": "2016-06-15T10:26:53Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20686",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20686#issuecomment-299106",
"user": "https://github.com/sagetrac-git"
}Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
9f529b0 | Improve getattr_from_other_class |
archive/issue_comments_299107.json:
{
"body": "Changed commit from **[`92f4520`](https://github.com/sagemath/sagetrac-mirror/commit/92f4520d01640379590d6c5974622f3d6f396e9b)** to **[`9f529b0`](https://github.com/sagemath/sagetrac-mirror/commit/9f529b0c4c8fd3523ebb557c9821e39799c41476)**",
"created_at": "2016-06-15T10:26:53Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20686",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20686#issuecomment-299107",
"user": "https://github.com/sagetrac-git"
}Changed commit from 92f4520 to 9f529b0
archive/issue_comments_299108.json:
{
"body": "<div id=\"comment:17\"></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/ff3a496289bbb6611d0feaf583d031522d69c90f\"><code>ff3a496</code></a></td><td><code>Improve getattr_from_other_class</code></td></tr></table>\n",
"created_at": "2016-06-15T11:52:18Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20686",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20686#issuecomment-299108",
"user": "https://github.com/sagetrac-git"
}Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
ff3a496 | Improve getattr_from_other_class |
archive/issue_comments_299109.json:
{
"body": "Changed commit from **[`9f529b0`](https://github.com/sagemath/sagetrac-mirror/commit/9f529b0c4c8fd3523ebb557c9821e39799c41476)** to **[`ff3a496`](https://github.com/sagemath/sagetrac-mirror/commit/ff3a496289bbb6611d0feaf583d031522d69c90f)**",
"created_at": "2016-06-15T11:52:18Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20686",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20686#issuecomment-299109",
"user": "https://github.com/sagetrac-git"
}Changed commit from 9f529b0 to ff3a496
archive/issue_events_288655.json:
{
"actor": "https://github.com/jdemeyer",
"created_at": "2016-06-15T11:52:57Z",
"event": "labeled",
"issue": "https://github.com/sagemath/sage/issues/20686",
"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/20686#event-288655"
}archive/issue_events_288656.json:
{
"actor": "https://github.com/jdemeyer",
"created_at": "2016-06-15T12:43:37Z",
"event": "renamed",
"issue": "https://github.com/sagemath/sage/issues/20686",
"title_is": "Refactor getattr_from_other_class() for lookup of methods in categories",
"title_was": "Optimize method lookup from the categories for instances of Cython classes",
"type": "issue_event",
"url": "https://github.com/sagemath/sage/issues/20686#event-288656"
}archive/issue_comments_299110.json:
{
"body": "Description changed:\n``````diff\n--- \n+++ \n@@ -1,4 +1,18 @@\n-For instances of Cython classes, the inheritance from categories is emulated by a manual lookup in `sage.structure.element.Element.__getattr__`. This lookup is about 10 times slower than a normal method lookup::\n+For parents and elements implemented in Cython, category lookup is emulated in `__getattr__` using `getattr_from_other_class`. In this ticket, this is refactored a bit:\n+\n+1. We should not pick up special attributes from `type`, there is no point in returning the type name of the category here (similarly for `__dict__`, `__mro__`, ...):\n+\n+```\n+sage: ZZ(1).__name__\n+'JoinCategory.element_class'\n+sage: ZZ.__name__\n+'JoinCategory.parent_class'\n+```\n+This change causes a few failures which need to be fixed.\n+\n+2. The implementation of `getattr_from_other_class` is not very robust. For example, static methods are not supported. We fix this by using low-level Python functions to get the attribute and we manually call the descriptor `__get__` if needed.\n+\n+3. The lookup using `getattr_from_other_class` is about 9 times slower than a normal method lookup::\n \n ```\n sage: def foo(self): return\n@@ -16,16 +30,6 @@\n sage: %timeit g(y)\n 1000 loops, best of 3: 1.18 ms per loop\n ```\n+We improve on this by roughly a factor 2 (but even then, it's still a lot slower than usual method lookup).\n \n-So there should be room for optimization (or using a different\n-approach for emulating this inheritance).\n-\n-Besides this, we should not pick up special attributes from `type`, there is no point in returning `type.__name__` here (similarly for `__dict__`, `__mro__`, ...):\n-\n-```\n-sage: getattr_from_other_class(1, type, \"__name__\")\n-'type'\n-```\n-This change causes a few failures which need to be fixed.\n-\n-This needs a trivial Cython patch (accepted by upstream): https://github.com/cython/cython/pull/522\n+NOTE: This needs a trivial Cython patch (accepted by upstream): https://github.com/cython/cython/pull/522\n``````\n",
"created_at": "2016-06-15T12:43:37Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20686",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20686#issuecomment-299110",
"user": "https://github.com/jdemeyer"
}Description changed:
---
+++
@@ -1,4 +1,18 @@
-For instances of Cython classes, the inheritance from categories is emulated by a manual lookup in `sage.structure.element.Element.__getattr__`. This lookup is about 10 times slower than a normal method lookup::
+For parents and elements implemented in Cython, category lookup is emulated in `__getattr__` using `getattr_from_other_class`. In this ticket, this is refactored a bit:
+
+1. We should not pick up special attributes from `type`, there is no point in returning the type name of the category here (similarly for `__dict__`, `__mro__`, ...):
+
+```
+sage: ZZ(1).__name__
+'JoinCategory.element_class'
+sage: ZZ.__name__
+'JoinCategory.parent_class'
+```
+This change causes a few failures which need to be fixed.
+
+2. The implementation of `getattr_from_other_class` is not very robust. For example, static methods are not supported. We fix this by using low-level Python functions to get the attribute and we manually call the descriptor `__get__` if needed.
+
+3. The lookup using `getattr_from_other_class` is about 9 times slower than a normal method lookup::
```
sage: def foo(self): return
@@ -16,16 +30,6 @@
sage: %timeit g(y)
1000 loops, best of 3: 1.18 ms per loop
```
+We improve on this by roughly a factor 2 (but even then, it's still a lot slower than usual method lookup).
-So there should be room for optimization (or using a different
-approach for emulating this inheritance).
-
-Besides this, we should not pick up special attributes from `type`, there is no point in returning `type.__name__` here (similarly for `__dict__`, `__mro__`, ...):
-
-```
-sage: getattr_from_other_class(1, type, "__name__")
-'type'
-```
-This change causes a few failures which need to be fixed.
-
-This needs a trivial Cython patch (accepted by upstream): https://github.com/cython/cython/pull/522
+NOTE: This needs a trivial Cython patch (accepted by upstream): https://github.com/cython/cython/pull/522archive/issue_comments_299111.json:
{
"body": "<div id=\"comment:20\"></div>\n\nBranch pushed to git repo; I updated commit sha1. New commits:\n<table><tr><td><a href=\"https://github.com/sagemath/sagetrac-mirror/commit/d49b67a34d416f17285c25d576c6444ad977f7b0\"><code>d49b67a</code></a></td><td><code>Test that static methods work</code></td></tr></table>\n",
"created_at": "2016-06-15T13:05:26Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20686",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20686#issuecomment-299111",
"user": "https://github.com/sagetrac-git"
}Branch pushed to git repo; I updated commit sha1. New commits:
d49b67a | Test that static methods work |
archive/issue_comments_299112.json:
{
"body": "Changed commit from **[`ff3a496`](https://github.com/sagemath/sagetrac-mirror/commit/ff3a496289bbb6611d0feaf583d031522d69c90f)** to **[`d49b67a`](https://github.com/sagemath/sagetrac-mirror/commit/d49b67a34d416f17285c25d576c6444ad977f7b0)**",
"created_at": "2016-06-15T13:05:26Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20686",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20686#issuecomment-299112",
"user": "https://github.com/sagetrac-git"
}Changed commit from ff3a496 to d49b67a
archive/issue_events_288657.json:
{
"actor": "https://github.com/jdemeyer",
"created_at": "2016-06-15T18:41:26Z",
"event": "unlabeled",
"issue": "https://github.com/sagemath/sage/issues/20686",
"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/20686#event-288657"
}archive/issue_events_288658.json:
{
"actor": "https://github.com/jdemeyer",
"created_at": "2016-06-15T18:41:26Z",
"event": "labeled",
"issue": "https://github.com/sagemath/sage/issues/20686",
"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/20686#event-288658"
}archive/issue_comments_299113.json:
{
"body": "<div id=\"comment:23\"></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/a9514e51ee9f0026440bf42632c7d618b3ad95fb\"><code>a9514e5</code></a></td><td><code>Improve getattr_from_other_class</code></td></tr><tr><td><a href=\"https://github.com/sagemath/sagetrac-mirror/commit/f91258699d2dc192702be2e142f51cf3dd03241d\"><code>f912586</code></a></td><td><code>Test that static methods work</code></td></tr><tr><td><a href=\"https://github.com/sagemath/sagetrac-mirror/commit/30594cce7e9bdb3e8a8ff5bfb601e93a52da175b\"><code>30594cc</code></a></td><td><code>Improve _sage_src_lines_()</code></td></tr></table>\n",
"created_at": "2016-06-15T19:17:35Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20686",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20686#issuecomment-299113",
"user": "https://github.com/sagetrac-git"
}Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
a9514e5 | Improve getattr_from_other_class |
f912586 | Test that static methods work |
30594cc | Improve _sage_src_lines_() |
archive/issue_comments_299114.json:
{
"body": "Changed commit from **[`d49b67a`](https://github.com/sagemath/sagetrac-mirror/commit/d49b67a34d416f17285c25d576c6444ad977f7b0)** to **[`30594cc`](https://github.com/sagemath/sagetrac-mirror/commit/30594cce7e9bdb3e8a8ff5bfb601e93a52da175b)**",
"created_at": "2016-06-15T19:17:35Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20686",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20686#issuecomment-299114",
"user": "https://github.com/sagetrac-git"
}Changed commit from d49b67a to 30594cc
archive/issue_comments_299115.json:
{
"body": "<div id=\"comment:24\"></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/851fa16587c1d74797b6ce66e101915e5ebbe339\"><code>851fa16</code></a></td><td><code>Improve _sage_src_lines_()</code></td></tr></table>\n",
"created_at": "2016-06-15T21:35:45Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20686",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20686#issuecomment-299115",
"user": "https://github.com/sagetrac-git"
}Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
851fa16 | Improve _sage_src_lines_() |
archive/issue_comments_299116.json:
{
"body": "Changed commit from **[`30594cc`](https://github.com/sagemath/sagetrac-mirror/commit/30594cce7e9bdb3e8a8ff5bfb601e93a52da175b)** to **[`851fa16`](https://github.com/sagemath/sagetrac-mirror/commit/851fa16587c1d74797b6ce66e101915e5ebbe339)**",
"created_at": "2016-06-15T21:35:45Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20686",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20686#issuecomment-299116",
"user": "https://github.com/sagetrac-git"
}Changed commit from 30594cc to 851fa16
archive/issue_comments_299117.json:
{
"body": "Changed commit from **[`851fa16`](https://github.com/sagemath/sagetrac-mirror/commit/851fa16587c1d74797b6ce66e101915e5ebbe339)** to **[`f5361d9`](https://github.com/sagemath/sagetrac-mirror/commit/f5361d972a61dcb60a76425264baa9e6d134e54b)**",
"created_at": "2016-06-16T07:39:34Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20686",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20686#issuecomment-299117",
"user": "https://github.com/sagetrac-git"
}Changed commit from 851fa16 to f5361d9
archive/issue_comments_299118.json:
{
"body": "<div id=\"comment:25\"></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/9a1ff6e69a71ab4ab5fe1a2775af9d083bc931b4\"><code>9a1ff6e</code></a></td><td><code>Improve getattr_from_other_class</code></td></tr><tr><td><a href=\"https://github.com/sagemath/sagetrac-mirror/commit/744ffa62fc2f8e193f28ab7f4e636c6edc947e67\"><code>744ffa6</code></a></td><td><code>Test that static methods work</code></td></tr><tr><td><a href=\"https://github.com/sagemath/sagetrac-mirror/commit/f5361d972a61dcb60a76425264baa9e6d134e54b\"><code>f5361d9</code></a></td><td><code>Improve _sage_src_lines_()</code></td></tr></table>\n",
"created_at": "2016-06-16T07:39:34Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20686",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20686#issuecomment-299118",
"user": "https://github.com/sagetrac-git"
}Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
9a1ff6e | Improve getattr_from_other_class |
744ffa6 | Test that static methods work |
f5361d9 | Improve _sage_src_lines_() |
archive/issue_comments_299119.json:
{
"body": "Changed keywords from none to **test**",
"created_at": "2016-06-17T14:32:13Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20686",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20686#issuecomment-299119",
"user": "https://github.com/embray"
}Changed keywords from none to test
archive/issue_comments_299120.json:
{
"body": "Changed keywords from **test** to none",
"created_at": "2016-06-17T14:32:27Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20686",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20686#issuecomment-299120",
"user": "https://github.com/embray"
}Changed keywords from test to none
archive/issue_comments_299121.json:
{
"body": "Description changed:\n``````diff\n--- \n+++ \n@@ -33,3 +33,5 @@\n We improve on this by roughly a factor 2 (but even then, it's still a lot slower than usual method lookup).\n \n NOTE: This needs a trivial Cython patch (accepted by upstream): https://github.com/cython/cython/pull/522\n+\n+Test\n``````\n",
"created_at": "2016-06-17T14:32:43Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20686",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20686#issuecomment-299121",
"user": "https://github.com/embray"
}Description changed:
---
+++
@@ -33,3 +33,5 @@
We improve on this by roughly a factor 2 (but even then, it's still a lot slower than usual method lookup).
NOTE: This needs a trivial Cython patch (accepted by upstream): https://github.com/cython/cython/pull/522
+
+Testarchive/issue_comments_299122.json:
{
"body": "Description changed:\n``````diff\n--- \n+++ \n@@ -33,5 +33,3 @@\n We improve on this by roughly a factor 2 (but even then, it's still a lot slower than usual method lookup).\n \n NOTE: This needs a trivial Cython patch (accepted by upstream): https://github.com/cython/cython/pull/522\n-\n-Test\n``````\n",
"created_at": "2016-06-17T14:33:01Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20686",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20686#issuecomment-299122",
"user": "https://github.com/embray"
}Description changed:
---
+++
@@ -33,5 +33,3 @@
We improve on this by roughly a factor 2 (but even then, it's still a lot slower than usual method lookup).
NOTE: This needs a trivial Cython patch (accepted by upstream): https://github.com/cython/cython/pull/522
-
-Testarchive/issue_comments_299123.json:
{
"body": "Description changed:\n``````diff\n--- \n+++ \n@@ -33,3 +33,5 @@\n We improve on this by roughly a factor 2 (but even then, it's still a lot slower than usual method lookup).\n \n NOTE: This needs a trivial Cython patch (accepted by upstream): https://github.com/cython/cython/pull/522\n+\n+Test\n``````\n",
"created_at": "2016-06-17T14:45:24Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20686",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20686#issuecomment-299123",
"user": "https://github.com/embray"
}Description changed:
---
+++
@@ -33,3 +33,5 @@
We improve on this by roughly a factor 2 (but even then, it's still a lot slower than usual method lookup).
NOTE: This needs a trivial Cython patch (accepted by upstream): https://github.com/cython/cython/pull/522
+
+Testarchive/issue_comments_299124.json:
{
"body": "<div id=\"comment:31\"></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/dc84d88add1dab3576723d2a7b20c8df4deb0151\"><code>dc84d88</code></a></td><td><code>Remove comments about \"private\" attributes with two underscores</code></td></tr><tr><td><a href=\"https://github.com/sagemath/sagetrac-mirror/commit/8a4f48d047a06ef9fcb2a0e01b5198f7ef439f9b\"><code>8a4f48d</code></a></td><td><code>Abstract away category lookup in cdef method getattr_from_category</code></td></tr></table>\n",
"created_at": "2016-06-20T13:12:41Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20686",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20686#issuecomment-299124",
"user": "https://github.com/sagetrac-git"
}Branch pushed to git repo; I updated commit sha1. New commits:
dc84d88 | Remove comments about "private" attributes with two underscores |
8a4f48d | Abstract away category lookup in cdef method getattr_from_category |
archive/issue_comments_299125.json:
{
"body": "Changed commit from **[`f5361d9`](https://github.com/sagemath/sagetrac-mirror/commit/f5361d972a61dcb60a76425264baa9e6d134e54b)** to **[`8a4f48d`](https://github.com/sagemath/sagetrac-mirror/commit/8a4f48d047a06ef9fcb2a0e01b5198f7ef439f9b)**",
"created_at": "2016-06-20T13:12:41Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20686",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20686#issuecomment-299125",
"user": "https://github.com/sagetrac-git"
}Changed commit from f5361d9 to 8a4f48d
archive/issue_comments_299126.json:
{
"body": "Changed branch from **[u/jdemeyer/optimize_method_lookup_from_the_categories_for_instances_of_cython_classes](https://github.com/sagemath/sagetrac-mirror/tree/u/jdemeyer/optimize_method_lookup_from_the_categories_for_instances_of_cython_classes)** to **[u/nthiery/optimize_method_lookup_from_the_categories_for_instances_of_cython_classes](https://github.com/sagemath/sagetrac-mirror/tree/u/nthiery/optimize_method_lookup_from_the_categories_for_instances_of_cython_classes)**",
"created_at": "2016-06-20T22:01:12Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20686",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20686#issuecomment-299126",
"user": "https://github.com/nthiery"
}Changed branch from u/jdemeyer/optimize_method_lookup_from_the_categories_for_instances_of_cython_classes to u/nthiery/optimize_method_lookup_from_the_categories_for_instances_of_cython_classes
archive/issue_comments_299127.json:
{
"body": "Changed branch from **[u/nthiery/optimize_method_lookup_from_the_categories_for_instances_of_cython_classes](https://github.com/sagemath/sagetrac-mirror/tree/u/nthiery/optimize_method_lookup_from_the_categories_for_instances_of_cython_classes)** to **[u/jdemeyer/optimize_method_lookup_from_the_categories_for_instances_of_cython_classes](https://github.com/sagemath/sagetrac-mirror/tree/u/jdemeyer/optimize_method_lookup_from_the_categories_for_instances_of_cython_classes)**",
"created_at": "2016-06-21T08:25:44Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20686",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20686#issuecomment-299127",
"user": "https://github.com/jdemeyer"
}Changed branch from u/nthiery/optimize_method_lookup_from_the_categories_for_instances_of_cython_classes to u/jdemeyer/optimize_method_lookup_from_the_categories_for_instances_of_cython_classes
archive/issue_comments_299128.json:
{
"body": "Changed commit from **[`8a4f48d`](https://github.com/sagemath/sagetrac-mirror/commit/8a4f48d047a06ef9fcb2a0e01b5198f7ef439f9b)** to **[`7140ee3`](https://github.com/sagemath/sagetrac-mirror/commit/7140ee389ac63921f356d5abc4cdc94a6bf48580)**",
"created_at": "2016-06-22T09:01:05Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20686",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20686#issuecomment-299128",
"user": "https://github.com/embray"
}Changed commit from 8a4f48d to 7140ee3
archive/issue_comments_299129.json:
{
"body": "<div id=\"comment:34\"></div>\n\nNew commits:\n<table><tr><td><a href=\"https://github.com/sagemath/sagetrac-mirror/commit/88d975794bc54532e785bfa9e337f70ee7dd8811\"><code>88d9757</code></a></td><td><code>Comment that getattr_from_other_class is cached</code></td></tr><tr><td><a href=\"https://github.com/sagemath/sagetrac-mirror/commit/7140ee389ac63921f356d5abc4cdc94a6bf48580\"><code>7140ee3</code></a></td><td><code>Prefer absolute imports</code></td></tr></table>\n",
"created_at": "2016-06-22T09:01:05Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20686",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20686#issuecomment-299129",
"user": "https://github.com/embray"
}New commits:
88d9757 | Comment that getattr_from_other_class is cached |
7140ee3 | Prefer absolute imports |
archive/issue_comments_299130.json:
{
"body": "Description changed:\n``````diff\n--- \n+++ \n@@ -33,5 +33,3 @@\n We improve on this by roughly a factor 2 (but even then, it's still a lot slower than usual method lookup).\n \n NOTE: This needs a trivial Cython patch (accepted by upstream): https://github.com/cython/cython/pull/522\n-\n-Test\n``````\n",
"created_at": "2016-06-22T09:01:05Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20686",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20686#issuecomment-299130",
"user": "https://github.com/embray"
}Description changed:
---
+++
@@ -33,5 +33,3 @@
We improve on this by roughly a factor 2 (but even then, it's still a lot slower than usual method lookup).
NOTE: This needs a trivial Cython patch (accepted by upstream): https://github.com/cython/cython/pull/522
-
-Testarchive/issue_events_288659.json:
{
"actor": "https://github.com/jdemeyer",
"created_at": "2016-06-22T09:03:55Z",
"event": "unlabeled",
"issue": "https://github.com/sagemath/sage/issues/20686",
"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/20686#event-288659"
}archive/issue_events_288660.json:
{
"actor": "https://github.com/jdemeyer",
"created_at": "2016-06-22T09:03:55Z",
"event": "labeled",
"issue": "https://github.com/sagemath/sage/issues/20686",
"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/20686#event-288660"
}archive/issue_comments_299131.json:
{
"body": "<div id=\"comment:36\" align=\"right\">comment:36</div>\n\nThanks Erik!",
"created_at": "2016-06-22T09:04:07Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20686",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20686#issuecomment-299131",
"user": "https://github.com/jdemeyer"
}comment:36
Thanks Erik!
archive/issue_comments_299132.json:
{
"body": "Description changed:\n``````diff\n--- \n+++ \n@@ -12,7 +12,11 @@\n \n 2. The implementation of `getattr_from_other_class` is not very robust. For example, static methods are not supported. We fix this by using low-level Python functions to get the attribute and we manually call the descriptor `__get__` if needed.\n \n-3. The lookup using `getattr_from_other_class` is about 9 times slower than a normal method lookup::\n+3. We shouldn't do anything special with double-underscore private attributes: in plain Python, this is implemented by the parser and not by `getattr()`. So `__getattr__` would already receive the mangled private name.\n+\n+4. Some of the changes broke `_sage_src_lines_`, so we also change that: currently, `_sage_src_lines_()` is used both to get the source lines of a class (e.g. dynamic classes) and an instance (e.g. cached functions). We change this to always mean the source lines of an instance, which makes things clearer.\n+\n+5. The lookup using `getattr_from_other_class` is about 9 times slower than a normal method lookup::\n \n ```\n sage: def foo(self): return\n``````\n",
"created_at": "2016-07-15T14:30:13Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20686",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20686#issuecomment-299132",
"user": "https://github.com/jdemeyer"
}Description changed:
---
+++
@@ -12,7 +12,11 @@
2. The implementation of `getattr_from_other_class` is not very robust. For example, static methods are not supported. We fix this by using low-level Python functions to get the attribute and we manually call the descriptor `__get__` if needed.
-3. The lookup using `getattr_from_other_class` is about 9 times slower than a normal method lookup::
+3. We shouldn't do anything special with double-underscore private attributes: in plain Python, this is implemented by the parser and not by `getattr()`. So `__getattr__` would already receive the mangled private name.
+
+4. Some of the changes broke `_sage_src_lines_`, so we also change that: currently, `_sage_src_lines_()` is used both to get the source lines of a class (e.g. dynamic classes) and an instance (e.g. cached functions). We change this to always mean the source lines of an instance, which makes things clearer.
+
+5. The lookup using `getattr_from_other_class` is about 9 times slower than a normal method lookup::
```
sage: def foo(self): returnarchive/issue_comments_299133.json:
{
"body": "Changed upstream from **Fixed upstream, but not in a stable release.** to **Fixed upstream, in a later stable release.**",
"created_at": "2016-07-15T14:35:12Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20686",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20686#issuecomment-299133",
"user": "https://github.com/jdemeyer"
}Changed upstream from Fixed upstream, but not in a stable release. to Fixed upstream, in a later stable release.
archive/issue_events_288661.json:
{
"actor": "https://github.com/jdemeyer",
"created_at": "2016-07-15T14:35:12Z",
"event": "unlabeled",
"issue": "https://github.com/sagemath/sage/issues/20686",
"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/20686#event-288661"
}archive/issue_events_288662.json:
{
"actor": "https://github.com/jdemeyer",
"created_at": "2016-07-15T14:35:12Z",
"event": "labeled",
"issue": "https://github.com/sagemath/sage/issues/20686",
"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/20686#event-288662"
}archive/issue_comments_299134.json:
{
"body": "Description changed:\n``````diff\n--- \n+++ \n@@ -36,4 +36,4 @@\n ```\n We improve on this by roughly a factor 2 (but even then, it's still a lot slower than usual method lookup).\n \n-NOTE: This needs a trivial Cython patch (accepted by upstream): https://github.com/cython/cython/pull/522\n+NOTE: This needs a trivial Cython patch, see #21030 for the Cython upgrade.\n``````\n",
"created_at": "2016-07-15T14:35:12Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20686",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20686#issuecomment-299134",
"user": "https://github.com/jdemeyer"
}Description changed:
---
+++
@@ -36,4 +36,4 @@
```
We improve on this by roughly a factor 2 (but even then, it's still a lot slower than usual method lookup).
-NOTE: This needs a trivial Cython patch (accepted by upstream): https://github.com/cython/cython/pull/522
+NOTE: This needs a trivial Cython patch, see #21030 for the Cython upgrade.archive/issue_comments_299135.json:
{
"body": "Changed dependencies from **#20825** to **#21030**",
"created_at": "2016-07-15T14:35:12Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20686",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20686#issuecomment-299135",
"user": "https://github.com/jdemeyer"
}Changed dependencies from #20825 to #21030
archive/issue_comments_299136.json:
{
"body": "<div id=\"comment:39\"></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/9cb4c3cea23a68e441780f3d109463618e195be5\"><code>9cb4c3c</code></a></td><td><code>Upgrade to Cython 0.24.1</code></td></tr><tr><td><a href=\"https://github.com/sagemath/sagetrac-mirror/commit/fc6487cb02514a811cb1f7ccfd92bed177bb2dea\"><code>fc6487c</code></a></td><td><code>Merge remote-tracking branch 'trac/u/jdemeyer/upgrade_to_cython_0_24_1' into t/20686/optimize_method_lookup_from_the_categories_for_instances_of_cython_classes</code></td></tr></table>\n",
"created_at": "2016-07-15T14:40:31Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20686",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20686#issuecomment-299136",
"user": "https://github.com/sagetrac-git"
}Branch pushed to git repo; I updated commit sha1. New commits:
9cb4c3c | Upgrade to Cython 0.24.1 |
fc6487c | Merge remote-tracking branch 'trac/u/jdemeyer/upgrade_to_cython_0_24_1' into t/20686/optimize_method_lookup_from_the_categories_for_instances_of_cython_classes |
archive/issue_comments_299137.json:
{
"body": "Changed commit from **[`7140ee3`](https://github.com/sagemath/sagetrac-mirror/commit/7140ee389ac63921f356d5abc4cdc94a6bf48580)** to **[`fc6487c`](https://github.com/sagemath/sagetrac-mirror/commit/fc6487cb02514a811cb1f7ccfd92bed177bb2dea)**",
"created_at": "2016-07-15T14:40:31Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20686",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20686#issuecomment-299137",
"user": "https://github.com/sagetrac-git"
}Changed commit from 7140ee3 to fc6487c
archive/issue_events_288663.json:
{
"actor": "https://github.com/jdemeyer",
"created_at": "2016-07-15T14:57:56Z",
"event": "unlabeled",
"issue": "https://github.com/sagemath/sage/issues/20686",
"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/20686#event-288663"
}archive/issue_events_288664.json:
{
"actor": "https://github.com/jdemeyer",
"created_at": "2016-07-15T14:57:56Z",
"event": "labeled",
"issue": "https://github.com/sagemath/sage/issues/20686",
"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/20686#event-288664"
}archive/issue_comments_299138.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/16d474363c051b081383dcdbc1944828e735c41e\"><code>16d4743</code></a></td><td><code>EvaluationMethods should be a new-style class</code></td></tr></table>\n",
"created_at": "2016-07-15T16:08:10Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20686",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20686#issuecomment-299138",
"user": "https://github.com/sagetrac-git"
}Branch pushed to git repo; I updated commit sha1. New commits:
16d4743 | EvaluationMethods should be a new-style class |
archive/issue_comments_299139.json:
{
"body": "Changed commit from **[`fc6487c`](https://github.com/sagemath/sagetrac-mirror/commit/fc6487cb02514a811cb1f7ccfd92bed177bb2dea)** to **[`16d4743`](https://github.com/sagemath/sagetrac-mirror/commit/16d474363c051b081383dcdbc1944828e735c41e)**",
"created_at": "2016-07-15T16:08:10Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20686",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20686#issuecomment-299139",
"user": "https://github.com/sagetrac-git"
}Changed commit from fc6487c to 16d4743
archive/issue_comments_299140.json:
{
"body": "Changed commit from **[`16d4743`](https://github.com/sagemath/sagetrac-mirror/commit/16d474363c051b081383dcdbc1944828e735c41e)** to **[`54ccd82`](https://github.com/sagemath/sagetrac-mirror/commit/54ccd828cb20b1254aeaf55bf7edab4a73f1032a)**",
"created_at": "2016-07-25T15:54:25Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20686",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20686#issuecomment-299140",
"user": "https://github.com/sagetrac-git"
}Changed commit from 16d4743 to 54ccd82
archive/issue_comments_299141.json:
{
"body": "<div id=\"comment:42\"></div>\n\nBranch pushed to git repo; I updated commit sha1. New commits:\n<table><tr><td><a href=\"https://github.com/sagemath/sagetrac-mirror/commit/54ccd828cb20b1254aeaf55bf7edab4a73f1032a\"><code>54ccd82</code></a></td><td><code>Remove `__dict__` attribute from ElementWrapper</code></td></tr></table>\n",
"created_at": "2016-07-25T15:54:25Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20686",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20686#issuecomment-299141",
"user": "https://github.com/sagetrac-git"
}Branch pushed to git repo; I updated commit sha1. New commits:
54ccd82 | Remove `__dict__` attribute from ElementWrapper |
archive/issue_comments_299142.json:
{
"body": "<div id=\"comment:43\" align=\"right\">comment:43</div>\n\nwhy is this test removed\n\n```diff\n- We test that \"private\" attributes are not requested from the element class\n- of the category (:trac:`10467`)::\n-\n- sage: C = EuclideanDomains()\n- sage: P.<x> = QQ[]\n- sage: C.element_class.__foo = 'bar'\n- sage: x.parent() in C\n- True\n- sage: x.__foo\n- Traceback (most recent call last):\n- ...\n- AttributeError: 'sage.rings.polynomial.polynomial_rational_flint.Polynomial_rational_flint' object has no attribute '__foo'\n```",
"created_at": "2016-07-25T22:14:03Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20686",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20686#issuecomment-299142",
"user": "https://github.com/videlec"
}comment:43
why is this test removed
- We test that "private" attributes are not requested from the element class
- of the category (:trac:`10467`)::
-
- sage: C = EuclideanDomains()
- sage: P.<x> = QQ[]
- sage: C.element_class.__foo = 'bar'
- sage: x.parent() in C
- True
- sage: x.__foo
- Traceback (most recent call last):
- ...
- AttributeError: 'sage.rings.polynomial.polynomial_rational_flint.Polynomial_rational_flint' object has no attribute '__foo'archive/issue_comments_299143.json:
{
"body": "<div id=\"comment:44\" align=\"right\">comment:44</div>\n\nsimilarly\n\n```diff\n- We test that \"private\" attributes are not requested from the element class\n- of the category (:trac:`10467`)::\n-\n- sage: P = Parent(QQ, category=CommutativeRings())\n- sage: P.category().parent_class.__foo = 'bar'\n- sage: P.__foo\n- Traceback (most recent call last):\n- ...\n- AttributeError: 'sage.structure.parent.Parent' object has no attribute '__foo'\n```",
"created_at": "2016-07-25T22:14:50Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20686",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20686#issuecomment-299143",
"user": "https://github.com/videlec"
}comment:44
similarly
- We test that "private" attributes are not requested from the element class
- of the category (:trac:`10467`)::
-
- sage: P = Parent(QQ, category=CommutativeRings())
- sage: P.category().parent_class.__foo = 'bar'
- sage: P.__foo
- Traceback (most recent call last):
- ...
- AttributeError: 'sage.structure.parent.Parent' object has no attribute '__foo'archive/issue_comments_299144.json:
{
"body": "<div id=\"comment:45\" align=\"right\">comment:45</div>\n\nReplying to [@videlec](#comment%3A43):\n> why is this test removed\n\nBecause #10467 was a mistake, see point 3 in the ticket description.",
"created_at": "2016-07-26T07:27:16Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20686",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20686#issuecomment-299144",
"user": "https://github.com/jdemeyer"
}comment:45
Replying to @videlec:
why is this test removed
Because #10467 was a mistake, see point 3 in the ticket description.
archive/issue_comments_299145.json:
{
"body": "<div id=\"comment:46\" align=\"right\">comment:46</div>\n\nMore precisely: see [#10467 comment:18](https://github.com/sagemath/sage/issues/10467#comment:18)",
"created_at": "2016-07-26T07:27:59Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20686",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20686#issuecomment-299145",
"user": "https://github.com/jdemeyer"
}comment:46
More precisely: see #10467 comment:18
archive/issue_comments_299146.json:
{
"body": "<div id=\"comment:47\" align=\"right\">comment:47</div>\n\nAargh....... this whole \"category lookup\" mechanism is causing me so much frustration with #20767. It is a very fragile hack which just happens to work correctly in the current setting but when you change anything in the coercion model it breaks.\n\nI am starting to think we should reconsider the whole category approach.",
"created_at": "2016-07-26T15:07:09Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20686",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20686#issuecomment-299146",
"user": "https://github.com/jdemeyer"
}comment:47
Aargh....... this whole "category lookup" mechanism is causing me so much frustration with #20767. It is a very fragile hack which just happens to work correctly in the current setting but when you change anything in the coercion model it breaks.
I am starting to think we should reconsider the whole category approach.
archive/issue_comments_299147.json:
{
"body": "<div id=\"comment:48\" align=\"right\">comment:48</div>\n\nIn case this could be useful, we could have a live chat about this tomorrow between 11pm and 2pm, or latter in the week.\n\nCheers,",
"created_at": "2016-07-26T21:14:58Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20686",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20686#issuecomment-299147",
"user": "https://github.com/nthiery"
}comment:48
In case this could be useful, we could have a live chat about this tomorrow between 11pm and 2pm, or latter in the week.
Cheers,
archive/issue_comments_299148.json:
{
"body": "<div id=\"comment:49\" align=\"right\">comment:49</div>\n\nReplying to [@nthiery](#comment%3A48):\n> In case this could be useful, we could have a live chat about this tomorrow between 11pm and 2pm, or latter in the week.\n\nYou meant 11am-2pm? Given that Santiago is -6h from Paris, it will be hard for me before 2pm (=8am in Santiago).",
"created_at": "2016-07-27T03:12:54Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20686",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20686#issuecomment-299148",
"user": "https://github.com/videlec"
}comment:49
Replying to @nthiery:
In case this could be useful, we could have a live chat about this tomorrow between 11pm and 2pm, or latter in the week.
You meant 11am-2pm? Given that Santiago is -6h from Paris, it will be hard for me before 2pm (=8am in Santiago).
archive/issue_comments_299149.json:
{
"body": "<div id=\"comment:50\" align=\"right\">comment:50</div>\n\nI am available most days during Paris daytime...",
"created_at": "2016-07-27T07:52:50Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20686",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20686#issuecomment-299149",
"user": "https://github.com/jdemeyer"
}comment:50
I am available most days during Paris daytime...
archive/issue_comments_299150.json:
{
"body": "<div id=\"comment:51\" align=\"right\">comment:51</div>\n\nAnyway, I am also hitting Cython bugs/oddities, so there are other battles to fight.",
"created_at": "2016-07-27T13:20:35Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20686",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20686#issuecomment-299150",
"user": "https://github.com/jdemeyer"
}comment:51
Anyway, I am also hitting Cython bugs/oddities, so there are other battles to fight.
archive/issue_comments_299151.json:
{
"body": "<div id=\"comment:52\" align=\"right\">comment:52</div>\n\nIn #20767, I \"solved\" the category problem (see [comment:47]) in the following way:\n\n- double-underscore methods (e.g. `__add__`) are never taken from the category.\n\n- anything deriving directly from `Element` fully supports single-underscore methods (e.g. `_add_`) from the category.\n\n- anything deriving from `ModuleElement`, `RingElement`,... has only partial support for single-underscore methods from the category.\n\n- nothing changes for non-arithmetic methods (e.g. `base_ring`).\n\nThe above rules do not break any existing usage of categories. I hope you can live with this...",
"created_at": "2016-08-03T11:00:42Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20686",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20686#issuecomment-299151",
"user": "https://github.com/jdemeyer"
}comment:52
In #20767, I "solved" the category problem (see [comment:47]) in the following way:
-
double-underscore methods (e.g.
__add__) are never taken from the category. -
anything deriving directly from
Elementfully supports single-underscore methods (e.g._add_) from the category. -
anything deriving from
ModuleElement,RingElement,... has only partial support for single-underscore methods from the category. -
nothing changes for non-arithmetic methods (e.g.
base_ring).
The above rules do not break any existing usage of categories. I hope you can live with this...
archive/issue_comments_299152.json:
{
"body": "<div id=\"comment:53\" align=\"right\">comment:53</div>\n\nThe following example of `getattr_from_other_class` line 249-253 in `sage/structure/misc.pyx` seems broken\n\n```\nsage: n = 1\nsage: ip = get_ipython()\nsage: ip.magic_psearch('n.N')\nTraceback (most recent call last):\n...\nAttributeError: 'SageTerminalInteractiveShell' object has no attribute 'magic_psearch'\n```",
"created_at": "2016-08-26T14:45:50Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20686",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20686#issuecomment-299152",
"user": "https://github.com/videlec"
}comment:53
The following example of getattr_from_other_class line 249-253 in sage/structure/misc.pyx seems broken
sage: n = 1
sage: ip = get_ipython()
sage: ip.magic_psearch('n.N')
Traceback (most recent call last):
...
AttributeError: 'SageTerminalInteractiveShell' object has no attribute 'magic_psearch'
archive/issue_comments_299153.json:
{
"body": "<div id=\"comment:54\" align=\"right\">comment:54</div>\n\nReplying to [@videlec](#comment%3A53):\n> The following example of `getattr_from_other_class` line 249-253 in `sage/structure/misc.pyx` seems broken\n\nTrue, but surely not because of this ticket. Most likely, it has to do with the IPython 5 upgrade.",
"created_at": "2016-08-26T15:27:33Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20686",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20686#issuecomment-299153",
"user": "https://github.com/jdemeyer"
}comment:54
Replying to @videlec:
The following example of
getattr_from_other_classline 249-253 insage/structure/misc.pyxseems broken
True, but surely not because of this ticket. Most likely, it has to do with the IPython 5 upgrade.
archive/issue_comments_299154.json:
{
"body": "<div id=\"comment:55\" align=\"right\">comment:55</div>\n\nIs that what we want\n\n```\nif self._category is None:\n # Usually, this will just raise AttributeError in\n # getattr_from_other_class().\n cls = type\nelse:\n cls = self._category.parent_class\n```\nIn other words, are there use case where the category is not initialized but we still might need to go through `__getattr__`?",
"created_at": "2016-08-26T16:00:46Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20686",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20686#issuecomment-299154",
"user": "https://github.com/videlec"
}comment:55
Is that what we want
if self._category is None:
# Usually, this will just raise AttributeError in
# getattr_from_other_class().
cls = type
else:
cls = self._category.parent_class
In other words, are there use case where the category is not initialized but we still might need to go through __getattr__?
archive/issue_comments_299155.json:
{
"body": "<div id=\"comment:56\" align=\"right\">comment:56</div>\n\nWhy in `Element` there is a two stage `__getattr__ -> getattr_from_category -> getattr_from_other_class` whereas in `Parent` there is no intermediate `getattr_from_category`?",
"created_at": "2016-08-26T16:04:17Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20686",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20686#issuecomment-299155",
"user": "https://github.com/videlec"
}comment:56
Why in Element there is a two stage __getattr__ -> getattr_from_category -> getattr_from_other_class whereas in Parent there is no intermediate getattr_from_category?
archive/issue_comments_299156.json:
{
"body": "<div id=\"comment:57\" align=\"right\">comment:57</div>\n\nReplying to [@videlec](#comment%3A56):\n> Why in `Element` there is a two stage `__getattr__ -> getattr_from_category -> getattr_from_other_class` whereas in `Parent` there is no intermediate `getattr_from_category`?\n\nMainly because I care more about `Element` than `Parent` (with #20767 in mind). The idea of `getattr_from_category` was to allow direct access to the category methods without normal methods. In the end, I am not using this in #20767 but I kept it because it looked potentially useful. And, since this a `cdef` method, there is essentially no performance penalty.",
"created_at": "2016-08-27T08:45:23Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20686",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20686#issuecomment-299156",
"user": "https://github.com/jdemeyer"
}comment:57
Replying to @videlec:
Why in
Elementthere is a two stage__getattr__ -> getattr_from_category -> getattr_from_other_classwhereas inParentthere is no intermediategetattr_from_category?
Mainly because I care more about Element than Parent (with #20767 in mind). The idea of getattr_from_category was to allow direct access to the category methods without normal methods. In the end, I am not using this in #20767 but I kept it because it looked potentially useful. And, since this a cdef method, there is essentially no performance penalty.
archive/issue_comments_299157.json:
{
"body": "<div id=\"comment:58\" align=\"right\">comment:58</div>\n\nReplying to [@videlec](#comment%3A55):\n> In other words, are there use case where the category is not initialized but we still might need to go through `__getattr__`?\n\nProbably not, but the old code also supported that (directly raising an `AttributeError`). But I just let `getattr_from_other_class` raise the error.",
"created_at": "2016-08-27T08:47:35Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20686",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20686#issuecomment-299157",
"user": "https://github.com/jdemeyer"
}comment:58
Replying to @videlec:
In other words, are there use case where the category is not initialized but we still might need to go through
__getattr__?
Probably not, but the old code also supported that (directly raising an AttributeError). But I just let getattr_from_other_class raise the error.
archive/issue_comments_299158.json:
{
"body": "<div id=\"comment:59\" align=\"right\">comment:59</div>\n\nReplying to [@jdemeyer](#comment%3A57):\n> Replying to [@videlec](#comment%3A56):\n> > Why in `Element` there is a two stage `__getattr__ -> getattr_from_category -> getattr_from_other_class` whereas in `Parent` there is no intermediate `getattr_from_category`?\n> \n> \n> Mainly because I care more about `Element` than `Parent` (with #20767 in mind). The idea of `getattr_from_category` was to allow direct access to the category methods without normal methods. In the end, I am not using this in #20767 but I kept it because it looked potentially useful. And, since this a `cdef` method, there is essentially no performance penalty.\n\nIt just looks weird to have twice the same thing implemented differently. I am just complaining about readability.",
"created_at": "2016-08-30T13:46:02Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20686",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20686#issuecomment-299158",
"user": "https://github.com/videlec"
}comment:59
Replying to @jdemeyer:
Replying to @videlec:
Why in
Elementthere is a two stage__getattr__ -> getattr_from_category -> getattr_from_other_classwhereas inParentthere is no intermediategetattr_from_category?Mainly because I care more about
ElementthanParent(with #20767 in mind). The idea ofgetattr_from_categorywas to allow direct access to the category methods without normal methods. In the end, I am not using this in #20767 but I kept it because it looked potentially useful. And, since this acdefmethod, there is essentially no performance penalty.
It just looks weird to have twice the same thing implemented differently. I am just complaining about readability.
archive/issue_comments_299159.json:
{
"body": "<div id=\"comment:60\" align=\"right\">comment:60</div>\n\nReplying to [@videlec](#comment%3A59):\n> It just looks weird to have twice the same thing implemented differently. I am just complaining about readability.\n\nOK, I will make the requested changes. I will also move the category lookup stuff from `Parent` to `CategoryObject` where it belongs.",
"created_at": "2016-08-31T07:58:40Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20686",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20686#issuecomment-299159",
"user": "https://github.com/jdemeyer"
}comment:60
Replying to @videlec:
It just looks weird to have twice the same thing implemented differently. I am just complaining about readability.
OK, I will make the requested changes. I will also move the category lookup stuff from Parent to CategoryObject where it belongs.
archive/issue_events_288665.json:
{
"actor": "https://github.com/jdemeyer",
"created_at": "2016-08-31T07:58:49Z",
"event": "unlabeled",
"issue": "https://github.com/sagemath/sage/issues/20686",
"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/20686#event-288665"
}archive/issue_events_288666.json:
{
"actor": "https://github.com/jdemeyer",
"created_at": "2016-08-31T07:58:49Z",
"event": "labeled",
"issue": "https://github.com/sagemath/sage/issues/20686",
"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/20686#event-288666"
}archive/issue_events_288667.json:
{
"actor": "https://github.com/jdemeyer",
"created_at": "2016-08-31T07:58:49Z",
"event": "demilestoned",
"issue": "https://github.com/sagemath/sage/issues/20686",
"milestone_number": null,
"milestone_title": "sage-7.3",
"type": "issue_event",
"url": "https://github.com/sagemath/sage/issues/20686#event-288667"
}archive/issue_events_288668.json:
{
"actor": "https://github.com/jdemeyer",
"created_at": "2016-08-31T07:58:49Z",
"event": "milestoned",
"issue": "https://github.com/sagemath/sage/issues/20686",
"milestone_number": null,
"milestone_title": "sage-7.4",
"type": "issue_event",
"url": "https://github.com/sagemath/sage/issues/20686#event-288668"
}archive/issue_comments_299160.json:
{
"body": "<div id=\"comment:62\"></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/a501048a8480fd66300c37ff614969469509d189\"><code>a501048</code></a></td><td><code>Move category attribute lookup to CategoryObject.getattr_from_category()</code></td></tr></table>\n",
"created_at": "2016-08-31T08:52:08Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20686",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20686#issuecomment-299160",
"user": "https://github.com/sagetrac-git"
}Branch pushed to git repo; I updated commit sha1. New commits:
a501048 | Move category attribute lookup to CategoryObject.getattr_from_category() |
archive/issue_comments_299161.json:
{
"body": "Changed commit from **[`54ccd82`](https://github.com/sagemath/sagetrac-mirror/commit/54ccd828cb20b1254aeaf55bf7edab4a73f1032a)** to **[`a501048`](https://github.com/sagemath/sagetrac-mirror/commit/a501048a8480fd66300c37ff614969469509d189)**",
"created_at": "2016-08-31T08:52:08Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20686",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20686#issuecomment-299161",
"user": "https://github.com/sagetrac-git"
}Changed commit from 54ccd82 to a501048
archive/issue_events_288669.json:
{
"actor": "https://github.com/jdemeyer",
"created_at": "2016-08-31T08:52:37Z",
"event": "unlabeled",
"issue": "https://github.com/sagemath/sage/issues/20686",
"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/20686#event-288669"
}archive/issue_events_288670.json:
{
"actor": "https://github.com/jdemeyer",
"created_at": "2016-08-31T08:52:37Z",
"event": "labeled",
"issue": "https://github.com/sagemath/sage/issues/20686",
"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/20686#event-288670"
}archive/issue_comments_299162.json:
{
"body": "<div id=\"comment:64\" align=\"right\">comment:64</div>\n\nDo you know whether it is worth it to keep this `__cached_method` dictionary? I would be happy to see timings here. (though this is rather disjoint from the main preoccupation of this ticket)",
"created_at": "2016-08-31T14:51:04Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20686",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20686#issuecomment-299162",
"user": "https://github.com/videlec"
}comment:64
Do you know whether it is worth it to keep this __cached_method dictionary? I would be happy to see timings here. (though this is rather disjoint from the main preoccupation of this ticket)
archive/issue_comments_299163.json:
{
"body": "<div id=\"comment:65\" align=\"right\">comment:65</div>\n\nIn `CategoryObject.__getattr__` it would better to do\n\n```\ntry:\n return self.__cached_methods[name]\nexcept KeyError:\n if self._category is None:\n cls = type\n else:\n cls = self._category.parent_class\n attr = getattr_from_other_class(self, cls, name)\n self.__cached_methods[name] = attr\n return attr\n```",
"created_at": "2016-08-31T14:53:27Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20686",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20686#issuecomment-299163",
"user": "https://github.com/videlec"
}comment:65
In CategoryObject.__getattr__ it would better to do
try:
return self.__cached_methods[name]
except KeyError:
if self._category is None:
cls = type
else:
cls = self._category.parent_class
attr = getattr_from_other_class(self, cls, name)
self.__cached_methods[name] = attr
return attr
archive/issue_comments_299164.json:
{
"body": "Reviewer: **Vincent Delecroix**",
"created_at": "2016-08-31T14:57:53Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20686",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20686#issuecomment-299164",
"user": "https://github.com/videlec"
}Reviewer: Vincent Delecroix
archive/issue_comments_299165.json:
{
"body": "Changed reviewer from **Vincent Delecroix** to none",
"created_at": "2016-08-31T14:58:21Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20686",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20686#issuecomment-299165",
"user": "https://github.com/jdemeyer"
}Changed reviewer from Vincent Delecroix to none
archive/issue_comments_299166.json:
{
"body": "<div id=\"comment:67\" align=\"right\">comment:67</div>\n\nReplying to [@videlec](#comment%3A64):\n> Do you know whether it is worth it to keep this `__cached_method` dictionary?\n\nI would guess so since a lookup in a dictionary should be faster than whatever `getattr_from_other_class` is doing.",
"created_at": "2016-08-31T14:58:21Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20686",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20686#issuecomment-299166",
"user": "https://github.com/jdemeyer"
}comment:67
Replying to @videlec:
Do you know whether it is worth it to keep this
__cached_methoddictionary?
I would guess so since a lookup in a dictionary should be faster than whatever getattr_from_other_class is doing.
archive/issue_comments_299167.json:
{
"body": "Reviewer: **Vincent Delecroix**",
"created_at": "2016-08-31T14:59:47Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20686",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20686#issuecomment-299167",
"user": "https://github.com/jdemeyer"
}Reviewer: Vincent Delecroix
archive/issue_events_288671.json:
{
"actor": "https://github.com/jdemeyer",
"created_at": "2016-08-31T14:59:47Z",
"event": "unlabeled",
"issue": "https://github.com/sagemath/sage/issues/20686",
"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/20686#event-288671"
}archive/issue_events_288672.json:
{
"actor": "https://github.com/jdemeyer",
"created_at": "2016-08-31T14:59:47Z",
"event": "labeled",
"issue": "https://github.com/sagemath/sage/issues/20686",
"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/20686#event-288672"
}archive/issue_comments_299168.json:
{
"body": "Changed commit from **[`a501048`](https://github.com/sagemath/sagetrac-mirror/commit/a501048a8480fd66300c37ff614969469509d189)** to **[`0fabb7a`](https://github.com/sagemath/sagetrac-mirror/commit/0fabb7a5dc50518946d1913488bf4a0444730c0f)**",
"created_at": "2016-08-31T16:34:49Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20686",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20686#issuecomment-299168",
"user": "https://github.com/sagetrac-git"
}Changed commit from a501048 to 0fabb7a
archive/issue_comments_299169.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/0fabb7a5dc50518946d1913488bf4a0444730c0f\"><code>0fabb7a</code></a></td><td><code>Optimize CategoryObject.getattr_from_category()</code></td></tr></table>\n",
"created_at": "2016-08-31T16:34:49Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20686",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20686#issuecomment-299169",
"user": "https://github.com/sagetrac-git"
}Branch pushed to git repo; I updated commit sha1. New commits:
0fabb7a | Optimize CategoryObject.getattr_from_category() |
archive/issue_events_288673.json:
{
"actor": "https://github.com/jdemeyer",
"created_at": "2016-08-31T16:36:18Z",
"event": "unlabeled",
"issue": "https://github.com/sagemath/sage/issues/20686",
"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/20686#event-288673"
}archive/issue_events_288674.json:
{
"actor": "https://github.com/jdemeyer",
"created_at": "2016-08-31T16:36:18Z",
"event": "labeled",
"issue": "https://github.com/sagemath/sage/issues/20686",
"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/20686#event-288674"
}archive/issue_events_288675.json:
{
"actor": "https://github.com/videlec",
"created_at": "2016-08-31T21:29:58Z",
"event": "unlabeled",
"issue": "https://github.com/sagemath/sage/issues/20686",
"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/20686#event-288675"
}archive/issue_events_288676.json:
{
"actor": "https://github.com/videlec",
"created_at": "2016-08-31T21:29:58Z",
"event": "labeled",
"issue": "https://github.com/sagemath/sage/issues/20686",
"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/20686#event-288676"
}archive/issue_comments_299170.json:
{
"body": "<div id=\"comment:71\" align=\"right\">comment:71</div>\n\nGot two failures with ptestlong\n\n```\nFile \"src/sage/combinat/root_system/integrable_representations.py\", line 189, in sage.combinat.root_system.integrable_representations.IntegrableRepresentation.__init__\nFailed example:\n TestSuite(V).run()\nExpected nothing\nGot:\n Failure in _test_additive_associativity:\n Traceback (most recent call last):\n ...\n AttributeError: 'IntegrableRepresentation' object has no attribute '_an_element_'\n ...\n```\nand similarly\n\n```\n**********************************************************************\nFile \"src/sage/homology/hochschild_complex.py\", line 98, in sage.homology.hochschild_complex.HochschildComplex.__init__\nFailed example:\n TestSuite(H).run(skip=\"_test_category\")\nExpected nothing\nGot:\n Failure in _test_additive_associativity:\n Traceback (most recent call last):\n ...\n AttributeError: 'HochschildComplex' object has no attribute '_an_element_'\n ...\n```",
"created_at": "2016-08-31T21:29:58Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20686",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20686#issuecomment-299170",
"user": "https://github.com/videlec"
}comment:71
Got two failures with ptestlong
File "src/sage/combinat/root_system/integrable_representations.py", line 189, in sage.combinat.root_system.integrable_representations.IntegrableRepresentation.__init__
Failed example:
TestSuite(V).run()
Expected nothing
Got:
Failure in _test_additive_associativity:
Traceback (most recent call last):
...
AttributeError: 'IntegrableRepresentation' object has no attribute '_an_element_'
...
and similarly
**********************************************************************
File "src/sage/homology/hochschild_complex.py", line 98, in sage.homology.hochschild_complex.HochschildComplex.__init__
Failed example:
TestSuite(H).run(skip="_test_category")
Expected nothing
Got:
Failure in _test_additive_associativity:
Traceback (most recent call last):
...
AttributeError: 'HochschildComplex' object has no attribute '_an_element_'
...
archive/issue_comments_299171.json:
{
"body": "<div id=\"comment:72\" align=\"right\">comment:72</div>\n\nAnd also not related to `_an_element_`\n\n```\n**********************************************************************\nFile \"src/sage/combinat/root_system/integrable_representations.py\", line 189, in sage.combinat.root_system.integrable_representations.IntegrableRepresentation.\n__init__\nFailed example:\n TestSuite(V).run()\nExpected nothing\nGot:\n ...\n Failure in _test_not_implemented_methods:\n Traceback (most recent call last):\n ...\n AssertionError: Not implemented method: __contains__\n ...\n```",
"created_at": "2016-08-31T21:31:51Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20686",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20686#issuecomment-299171",
"user": "https://github.com/videlec"
}comment:72
And also not related to _an_element_
**********************************************************************
File "src/sage/combinat/root_system/integrable_representations.py", line 189, in sage.combinat.root_system.integrable_representations.IntegrableRepresentation.
__init__
Failed example:
TestSuite(V).run()
Expected nothing
Got:
...
Failure in _test_not_implemented_methods:
Traceback (most recent call last):
...
AssertionError: Not implemented method: __contains__
...
archive/issue_comments_299172.json:
{
"body": "<div id=\"comment:73\" align=\"right\">comment:73</div>\n\nThe last patchbot report was green, so this must be a recently-introduced problem.",
"created_at": "2016-09-01T07:22:11Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20686",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20686#issuecomment-299172",
"user": "https://github.com/jdemeyer"
}comment:73
The last patchbot report was green, so this must be a recently-introduced problem.
archive/issue_comments_299173.json:
{
"body": "<div id=\"comment:74\" align=\"right\">comment:74</div>\n\nInterestingly, these failures are for classes inheriting from `CategoryObject` but not `Parent`.",
"created_at": "2016-09-01T08:21:04Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20686",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20686#issuecomment-299173",
"user": "https://github.com/jdemeyer"
}comment:74
Interestingly, these failures are for classes inheriting from CategoryObject but not Parent.
archive/issue_comments_299174.json:
{
"body": "<div id=\"comment:75\" align=\"right\">comment:75</div>\n\nThose failures are true bugs in `HochschildComplex` and `IntegrableRepresentation`: they pretend to be in some category but do not actually implement everything needed to be in that category. This ticket causes the category tests to be run (which is good) but then the tests fail.\n\n```\nsage: SGA = SymmetricGroupAlgebra(QQ, 3)\nsage: T = SGA.trivial_representation()\nsage: H = SGA.hochschild_complex(T)\nsage: H in CommutativeAdditiveGroups()\nTrue\nsage: H.zero()\n...\nTypeError: 'HochschildComplex' object is not callable\n```\n\nand\n\n```\nsage: Lambda = RootSystem(['A',3,1]).weight_lattice(extended=true).fundamental_weights()\nsage: V = IntegrableRepresentation(Lambda[1]+Lambda[2]+Lambda[3])\nsage: V in CommutativeAdditiveGroups()\nTrue\nsage: V.zero()\n...\nTypeError: 'IntegrableRepresentation' object is not callable\n```\n\nI think the right thing to do here is to mark those tests as `# known bug`.",
"created_at": "2016-09-01T08:32:07Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20686",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20686#issuecomment-299174",
"user": "https://github.com/jdemeyer"
}comment:75
Those failures are true bugs in HochschildComplex and IntegrableRepresentation: they pretend to be in some category but do not actually implement everything needed to be in that category. This ticket causes the category tests to be run (which is good) but then the tests fail.
sage: SGA = SymmetricGroupAlgebra(QQ, 3)
sage: T = SGA.trivial_representation()
sage: H = SGA.hochschild_complex(T)
sage: H in CommutativeAdditiveGroups()
True
sage: H.zero()
...
TypeError: 'HochschildComplex' object is not callable
and
sage: Lambda = RootSystem(['A',3,1]).weight_lattice(extended=true).fundamental_weights()
sage: V = IntegrableRepresentation(Lambda[1]+Lambda[2]+Lambda[3])
sage: V in CommutativeAdditiveGroups()
True
sage: V.zero()
...
TypeError: 'IntegrableRepresentation' object is not callable
I think the right thing to do here is to mark those tests as # known bug.
archive/issue_comments_299175.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/74041f30d8de4c15055345f14340a4f443236ffa\"><code>74041f3</code></a></td><td><code>Mark broken testsuites as # known bug</code></td></tr></table>\n",
"created_at": "2016-09-01T08:54:28Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20686",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20686#issuecomment-299175",
"user": "https://github.com/sagetrac-git"
}Branch pushed to git repo; I updated commit sha1. New commits:
74041f3 | Mark broken testsuites as # known bug |
archive/issue_comments_299176.json:
{
"body": "Changed commit from **[`0fabb7a`](https://github.com/sagemath/sagetrac-mirror/commit/0fabb7a5dc50518946d1913488bf4a0444730c0f)** to **[`74041f3`](https://github.com/sagemath/sagetrac-mirror/commit/74041f30d8de4c15055345f14340a4f443236ffa)**",
"created_at": "2016-09-01T08:54:28Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20686",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20686#issuecomment-299176",
"user": "https://github.com/sagetrac-git"
}Changed commit from 0fabb7a to 74041f3
archive/issue_comments_299177.json:
{
"body": "<div id=\"comment:77\" align=\"right\">comment:77</div>\n\nI created #21386 and #21387 for the broken testsuites.",
"created_at": "2016-09-01T08:55:16Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20686",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20686#issuecomment-299177",
"user": "https://github.com/jdemeyer"
}comment:77
I created #21386 and #21387 for the broken testsuites.
archive/issue_events_288677.json:
{
"actor": "https://github.com/jdemeyer",
"created_at": "2016-09-01T08:55:16Z",
"event": "unlabeled",
"issue": "https://github.com/sagemath/sage/issues/20686",
"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/20686#event-288677"
}archive/issue_events_288678.json:
{
"actor": "https://github.com/jdemeyer",
"created_at": "2016-09-01T08:55:16Z",
"event": "labeled",
"issue": "https://github.com/sagemath/sage/issues/20686",
"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/20686#event-288678"
}archive/issue_comments_299178.json:
{
"body": "<div id=\"comment:78\" align=\"right\">comment:78</div>\n\nI hope to fix both today.",
"created_at": "2016-09-01T13:52:36Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20686",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20686#issuecomment-299178",
"user": "https://github.com/tscrim"
}comment:78
I hope to fix both today.
archive/issue_comments_299179.json:
{
"body": "<div id=\"comment:79\" align=\"right\">comment:79</div>\n\ngood to go!",
"created_at": "2016-09-01T14:22:12Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20686",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20686#issuecomment-299179",
"user": "https://github.com/videlec"
}comment:79
good to go!
archive/issue_events_288679.json:
{
"actor": "https://github.com/videlec",
"created_at": "2016-09-01T14:22:12Z",
"event": "unlabeled",
"issue": "https://github.com/sagemath/sage/issues/20686",
"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/20686#event-288679"
}archive/issue_events_288680.json:
{
"actor": "https://github.com/videlec",
"created_at": "2016-09-01T14:22:12Z",
"event": "labeled",
"issue": "https://github.com/sagemath/sage/issues/20686",
"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/20686#event-288680"
}archive/issue_comments_299180.json:
{
"body": "Changed dependencies from **#21030** to **#21030, #21409**",
"created_at": "2016-09-03T15:11:26Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20686",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20686#issuecomment-299180",
"user": "https://github.com/vbraun"
}Changed dependencies from #21030 to #21030, #21409
archive/issue_comments_299181.json:
{
"body": "<div id=\"comment:80\" align=\"right\">comment:80</div>\n\nThis ticket seems to cause #21409",
"created_at": "2016-09-03T15:11:26Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20686",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20686#issuecomment-299181",
"user": "https://github.com/vbraun"
}comment:80
This ticket seems to cause #21409
archive/issue_comments_299182.json:
{
"body": "Changed branch from **[u/jdemeyer/optimize_method_lookup_from_the_categories_for_instances_of_cython_classes](https://github.com/sagemath/sagetrac-mirror/tree/u/jdemeyer/optimize_method_lookup_from_the_categories_for_instances_of_cython_classes)** to **[`74041f3`](https://github.com/sagemath/sagetrac-mirror/commit/74041f30d8de4c15055345f14340a4f443236ffa)**",
"created_at": "2016-09-04T13:38:02Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20686",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20686#issuecomment-299182",
"user": "https://github.com/vbraun"
}Changed branch from u/jdemeyer/optimize_method_lookup_from_the_categories_for_instances_of_cython_classes to 74041f3
archive/issue_events_288681.json:
{
"actor": "https://github.com/vbraun",
"created_at": "2016-09-04T13:38:02Z",
"event": "unlabeled",
"issue": "https://github.com/sagemath/sage/issues/20686",
"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/20686#event-288681"
}archive/issue_events_288682.json:
{
"actor": "https://github.com/vbraun",
"commit_id": "c95a93fb737a94e852b83a07ff94c5220674c151",
"commit_repository": "https://github.com/sagemath/sage",
"created_at": "2016-09-04T13:38:02Z",
"event": "closed",
"issue": "https://github.com/sagemath/sage/issues/20686",
"type": "issue_event",
"url": "https://github.com/sagemath/sage/issues/20686#event-288682"
}