archive/issues_020151.json:
{
"assignees": [],
"body": "<div id=\"comment:0\"></div>\n\nI have frequently encountered computer setups where one has access to a Sage installation locally, but Magma is only installed in a remote machine (say with address 'remote'). Typically one either:\n1) Logs into 'remote' via ssh and does some computation in Magma.\n2) uses the ssh command\n\n```\n $> ssh -t remote 'magma'\n```\nto pretend having magma installed locally.\n\nActually Sage has functionality to use (2) in its interface with Magma, but it is currently broken and not advertised. It presumes that 'remote' also has sage installed (in fact it presumes that sage-native-execute is in the default PATH), for example.\n\nThis ticket fixes this, and makes a command like\n\n```\n sage: magma = Magma(server = 'remote', command = 'magma-myversion')\n```\nwork, and provide the user with an interface to a particular version of Magma installed in 'remote'.\n\nComponent: **interfaces: optional**\n\nKeywords: **magma, remote**\n\nAuthor: **Marc Masdeu**\n\nBranch/Commit: **[`f499648`](https://github.com/sagemath/sagetrac-mirror/commit/f4996488101ac3ce85550fb14b2eec08185408a6)**\n\nReviewer: **Nils Bruin, Vincent Delecroix**\n\n_Issue created by migration from https://trac.sagemath.org/ticket/20388_\n\n",
"closed_at": "2016-04-16T10:25:59Z",
"created_at": "2016-04-08T12:01:16Z",
"labels": [
"https://github.com/sagemath/sage/labels/c%3A%20interfaces%3A%20optional",
"https://github.com/sagemath/sage/labels/p%3A%20major%20/%203",
"https://github.com/sagemath/sage/labels/bug"
],
"milestone": "https://github.com/sagemath/sage/milestones/sage-7.2",
"reactions": [],
"repository": "https://github.com/sagemath/sage",
"title": "Fix the Magma interface to work with remote installations",
"type": "issue",
"updated_at": "2016-04-16T10:25:59Z",
"url": "https://github.com/sagemath/sage/issues/20388",
"user": "https://github.com/mmasdeu"
}I have frequently encountered computer setups where one has access to a Sage installation locally, but Magma is only installed in a remote machine (say with address 'remote'). Typically one either:
- Logs into 'remote' via ssh and does some computation in Magma.
- uses the ssh command
$> ssh -t remote 'magma'
to pretend having magma installed locally.
Actually Sage has functionality to use (2) in its interface with Magma, but it is currently broken and not advertised. It presumes that 'remote' also has sage installed (in fact it presumes that sage-native-execute is in the default PATH), for example.
This ticket fixes this, and makes a command like
sage: magma = Magma(server = 'remote', command = 'magma-myversion')
work, and provide the user with an interface to a particular version of Magma installed in 'remote'.
Component: interfaces: optional
Keywords: magma, remote
Author: Marc Masdeu
Branch/Commit: f499648
Reviewer: Nils Bruin, Vincent Delecroix
Issue created by migration from https://trac.sagemath.org/ticket/20388
archive/issue_events_285074.json:
{
"actor": "https://github.com/mmasdeu",
"created_at": "2016-04-08T12:01:16Z",
"event": "milestoned",
"issue": "https://github.com/sagemath/sage/issues/20388",
"milestone_number": null,
"milestone_title": "sage-7.2",
"type": "issue_event",
"url": "https://github.com/sagemath/sage/issues/20388#event-285074"
}archive/issue_events_285075.json:
{
"actor": "https://github.com/mmasdeu",
"created_at": "2016-04-08T12:01:16Z",
"event": "labeled",
"issue": "https://github.com/sagemath/sage/issues/20388",
"label": "https://github.com/sagemath/sage/labels/c%3A%20interfaces%3A%20optional",
"label_color": "0000ff",
"label_name": "c: interfaces: optional",
"label_text_color": "ffffff",
"type": "issue_event",
"url": "https://github.com/sagemath/sage/issues/20388#event-285075"
}archive/issue_events_285076.json:
{
"actor": "https://github.com/mmasdeu",
"created_at": "2016-04-08T12:01:16Z",
"event": "labeled",
"issue": "https://github.com/sagemath/sage/issues/20388",
"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/20388#event-285076"
}archive/issue_events_285077.json:
{
"actor": "https://github.com/mmasdeu",
"created_at": "2016-04-08T12:01:16Z",
"event": "labeled",
"issue": "https://github.com/sagemath/sage/issues/20388",
"label": "https://github.com/sagemath/sage/labels/bug",
"label_color": "d73a4a",
"label_name": "bug",
"label_text_color": "ffffff",
"type": "issue_event",
"url": "https://github.com/sagemath/sage/issues/20388#event-285077"
}archive/issue_events_285078.json:
{
"actor": "https://github.com/mmasdeu",
"created_at": "2016-04-08T12:02:36Z",
"event": "labeled",
"issue": "https://github.com/sagemath/sage/issues/20388",
"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/20388#event-285078"
}archive/issue_comments_294337.json:
{
"body": "Branch: **[u/mmasdeu/20388](https://github.com/sagemath/sagetrac-mirror/tree/u/mmasdeu/20388)**",
"created_at": "2016-04-08T12:02:36Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20388",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20388#issuecomment-294337",
"user": "https://github.com/mmasdeu"
}Branch: u/mmasdeu/20388
archive/issue_comments_294338.json:
{
"body": "<div id=\"comment:1\"></div>\n\nNew commits:\n<table><tr><td><a href=\"https://github.com/sagemath/sagetrac-mirror/commit/8cf2e2a12bd95413c4ea44d042f40261cf10b567\"><code>8cf2e2a</code></a></td><td><code>Fixed Magma interface to work with remote server.</code></td></tr><tr><td><a href=\"https://github.com/sagemath/sagetrac-mirror/commit/5fb6df4fb2bae17c681c34aac2d15a66f540f006\"><code>5fb6df4</code></a></td><td><code>Fixed spacing.</code></td></tr><tr><td><a href=\"https://github.com/sagemath/sagetrac-mirror/commit/ec0a724df6548f328d0947ad166c12d96392d74e\"><code>ec0a724</code></a></td><td><code>Added message to help in discovering the feature.</code></td></tr></table>\n",
"created_at": "2016-04-08T12:02:36Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20388",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20388#issuecomment-294338",
"user": "https://github.com/mmasdeu"
}New commits:
8cf2e2a | Fixed Magma interface to work with remote server. |
5fb6df4 | Fixed spacing. |
ec0a724 | Added message to help in discovering the feature. |
archive/issue_comments_294339.json:
{
"body": "Commit: **[`ec0a724`](https://github.com/sagemath/sagetrac-mirror/commit/ec0a724df6548f328d0947ad166c12d96392d74e)**",
"created_at": "2016-04-08T12:02:36Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20388",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20388#issuecomment-294339",
"user": "https://github.com/mmasdeu"
}Commit: ec0a724
archive/issue_comments_294340.json:
{
"body": "<div id=\"comment:2\" align=\"right\">comment:2</div>\n\nI was able to use it! Note that on the server I have access to I need two commands to launch magma\n\n```\n$ module load magma/2.11.13 # Slurm\n$ magma\n```\nAnd `Magma(server='plafrim', command='module load magma/2.11.13; magma')` worked perfectly. Do you think it is worth it to add in the doc?\n\n**Sided note:** Would be nice for users to set up their configurations once for all for these external interfaces (via environment variables?). For example, I have access to maple and magma but they live on two different servers.",
"created_at": "2016-04-11T17:02:42Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20388",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20388#issuecomment-294340",
"user": "https://github.com/videlec"
}I was able to use it! Note that on the server I have access to I need two commands to launch magma
$ module load magma/2.11.13 # Slurm
$ magma
And Magma(server='plafrim', command='module load magma/2.11.13; magma') worked perfectly. Do you think it is worth it to add in the doc?
Sided note: Would be nice for users to set up their configurations once for all for these external interfaces (via environment variables?). For example, I have access to maple and magma but they live on two different servers.
archive/issue_comments_294341.json:
{
"body": "<div id=\"comment:3\" align=\"right\">comment:3</div>\n\nThat's great! I added the 'command' parameter because it could be potentially useful although I didn't need it. I am happy to see that my guess was correct.\n\nAbout setting defaults, have you tried to put\n\n```\nmagma = Magma(server='plafrim', command='module load magma/2.11.13; magma')\nmaple = (whatever is pertinent, I don't have access to maple)\n```\nin $HOME/.sage/init.sage?\n\nFinally, although I agree that the documentation should advertise more this feature, I don't know where to put it exactly.",
"created_at": "2016-04-11T17:13:27Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20388",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20388#issuecomment-294341",
"user": "https://github.com/mmasdeu"
}That's great! I added the 'command' parameter because it could be potentially useful although I didn't need it. I am happy to see that my guess was correct.
About setting defaults, have you tried to put
magma = Magma(server='plafrim', command='module load magma/2.11.13; magma')
maple = (whatever is pertinent, I don't have access to maple)
in $HOME/.sage/init.sage?
Finally, although I agree that the documentation should advertise more this feature, I don't know where to put it exactly.
archive/issue_comments_294342.json:
{
"body": "<div id=\"comment:4\" align=\"right\">comment:4</div>\n\nSetting them in `init.sage` is indeed a solution to make `magma(something)` works out of the box.\n\nMy question about default configuration did not make much sense since I thought that `SageObject` would have methods `_magma_` and `_maple_`. If it was\n\n```\n def _magma_(self, G=None):\n if G is None:\n import sage.interfaces.magma\n G = sage.interfaces.magma.magma\n return self._interface_(G)\n```\nthen it would not take into account any specific server configuration.",
"created_at": "2016-04-11T17:29:38Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20388",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20388#issuecomment-294342",
"user": "https://github.com/videlec"
}Setting them in init.sage is indeed a solution to make magma(something) works out of the box.
My question about default configuration did not make much sense since I thought that SageObject would have methods _magma_ and _maple_. If it was
def _magma_(self, G=None):
if G is None:
import sage.interfaces.magma
G = sage.interfaces.magma.magma
return self._interface_(G)
then it would not take into account any specific server configuration.
archive/issue_comments_294343.json:
{
"body": "<div id=\"comment:5\"></div>\n\nBranch pushed to git repo; I updated commit sha1. New commits:\n<table><tr><td><a href=\"https://github.com/sagemath/sagetrac-mirror/commit/da0c8a128c83b89f9cc6f42b3dfcdcac3f8fce24\"><code>da0c8a1</code></a></td><td><code>Fixed two functions that were using the default magma.</code></td></tr></table>\n",
"created_at": "2016-04-11T17:51:48Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20388",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20388#issuecomment-294343",
"user": "https://github.com/sagetrac-git"
}Branch pushed to git repo; I updated commit sha1. New commits:
da0c8a1 | Fixed two functions that were using the default magma. |
archive/issue_comments_294344.json:
{
"body": "Changed commit from **[`ec0a724`](https://github.com/sagemath/sagetrac-mirror/commit/ec0a724df6548f328d0947ad166c12d96392d74e)** to **[`da0c8a1`](https://github.com/sagemath/sagetrac-mirror/commit/da0c8a128c83b89f9cc6f42b3dfcdcac3f8fce24)**",
"created_at": "2016-04-11T17:51:48Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20388",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20388#issuecomment-294344",
"user": "https://github.com/sagetrac-git"
}Changed commit from ec0a724 to da0c8a1
archive/issue_comments_294345.json:
{
"body": "<div id=\"comment:6\" align=\"right\">comment:6</div>\n\nThanks for testing it! I found two functions (one in arith.all, one in plane_conics/con_field.py) that imported magma. I have changed them to try to use the global magma if it exists (which should be the case if the user initialized it, anyway).",
"created_at": "2016-04-11T17:53:48Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20388",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20388#issuecomment-294345",
"user": "https://github.com/mmasdeu"
}Thanks for testing it! I found two functions (one in arith.all, one in plane_conics/con_field.py) that imported magma. I have changed them to try to use the global magma if it exists (which should be the case if the user initialized it, anyway).
archive/issue_comments_294346.json:
{
"body": "<div id=\"comment:7\" align=\"right\">comment:7</div>\n\nA few things you might want to look at:\n- sage's pexpect interfaces have an option to use files for transfer if the input is large. That's going it be an issue, because in your scenario you wouldn't want to assume sage and (remote) magma have access to a shared file system. So you might have to turn this capability off.\n- You're now assuming that the remote magma user has permission to do an \"scp\" to the remote server. Indeed, the way we currently have written `ext/magma` requires file access (it's written as a module). However, one could put those files within reach of magma via other means. The \"scp\" variant possibly works quite often, so it may be a reasonable default thing to try, but we should probably not fail if it doesn't work.",
"created_at": "2016-04-11T18:08:38Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20388",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20388#issuecomment-294346",
"user": "https://github.com/nbruin"
}A few things you might want to look at:
- sage's pexpect interfaces have an option to use files for transfer if the input is large. That's going it be an issue, because in your scenario you wouldn't want to assume sage and (remote) magma have access to a shared file system. So you might have to turn this capability off.
- You're now assuming that the remote magma user has permission to do an "scp" to the remote server. Indeed, the way we currently have written
ext/magmarequires file access (it's written as a module). However, one could put those files within reach of magma via other means. The "scp" variant possibly works quite often, so it may be a reasonable default thing to try, but we should probably not fail if it doesn't work.
archive/issue_comments_294347.json:
{
"body": "<div id=\"comment:8\" align=\"right\">comment:8</div>\n\nRelying on `globals` is not very safe\n\n```\nsage: magma = MyMagmaStructure()\nsage: bernoulli(3, algorithm='magma') # boom\n```\n\nI think that the magma interface (and all interface in general) should just have some default configurable parameters. That would be used with something like\n\n```\nsage: sage.interfaces.config('magma', server=X', command='Y')\n```",
"created_at": "2016-04-11T18:12:23Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20388",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20388#issuecomment-294347",
"user": "https://github.com/videlec"
}Relying on globals is not very safe
sage: magma = MyMagmaStructure()
sage: bernoulli(3, algorithm='magma') # boom
I think that the magma interface (and all interface in general) should just have some default configurable parameters. That would be used with something like
sage: sage.interfaces.config('magma', server=X', command='Y')
archive/issue_comments_294348.json:
{
"body": "<div id=\"comment:9\" align=\"right\">comment:9</div>\n\nReplying to [@videlec](#comment%3A8):\n> I think that the magma interface (and all interface in general) should just have some default configurable parameters. That would be used with something like\n> \n> ```\n> sage: sage.interfaces.config('magma', server=X', command='Y')\n> ```\n\nI would hang those properties on `sage.interfaces.magma.magma`, which is the \"default\" instantiation of the magma interface.\n\nNote that `sage.interfaces.magma.magma` is instantiated upon load, so we're too late to set defaults for its instantiation. However, actually *starting up* only happens once the interface really gets used, so you could just adapt the relevant properties on the magma object before using the interface.\n\nA workaround you can already do if you want to use a magma with non-standard setting is \"monkey patch\" the global instance, i.e.,\n\n```\nsage: sage.interfaces.magma.magma = Magma(...<options you want>...)\n```\nwith the problem that if some other file (such as `sage.interfaces.all`) has done an `import from`, you're not reaching them. So perhaps it's a little more robust to adjust attributes on `sage.interfaces.magma.magma` rather than rebind it to a different instance (sigh ... and then they say you don't have to think about pointers when working in python)",
"created_at": "2016-04-11T20:51:55Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20388",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20388#issuecomment-294348",
"user": "https://github.com/nbruin"
}Replying to @videlec:
I think that the magma interface (and all interface in general) should just have some default configurable parameters. That would be used with something like
sage: sage.interfaces.config('magma', server=X', command='Y')
I would hang those properties on sage.interfaces.magma.magma, which is the "default" instantiation of the magma interface.
Note that sage.interfaces.magma.magma is instantiated upon load, so we're too late to set defaults for its instantiation. However, actually starting up only happens once the interface really gets used, so you could just adapt the relevant properties on the magma object before using the interface.
A workaround you can already do if you want to use a magma with non-standard setting is "monkey patch" the global instance, i.e.,
sage: sage.interfaces.magma.magma = Magma(...<options you want>...)
with the problem that if some other file (such as sage.interfaces.all) has done an import from, you're not reaching them. So perhaps it's a little more robust to adjust attributes on sage.interfaces.magma.magma rather than rebind it to a different instance (sigh ... and then they say you don't have to think about pointers when working in python)
archive/issue_comments_294349.json:
{
"body": "<div id=\"comment:10\" align=\"right\">comment:10</div>\n\nI have addressed Nils' two concerns above. About the \"magma = Magma()\" and the configuration parameters, I don't have a quick solution yet. There's a bunch of places in the schemes/ folder that import magma depending on the algorithm chosen. One easy way to fix this is to provide a get_magma_session() and set_magma_session() functions so that something like this would work:\n\n```\nsage: set_magma_session(Magma(server = 'remote', command = 'magma_custom'))\nsage: function_using_magma(algorithm = 'magma')\n```\n\nThis would only involve changing the calls that look like:\n\n```\nfrom sage.interfaces.magma import magma\nmagma.eval('complicated stuff')\n```\nto\n\n```\nfrom sage.interfaces.magma import get_magma_session\nmagma = get_magma_session()\nmagma.eval('complicated stuff')\n```\n\nDo you think that this is a good enough solution? If so, I would quickly implement it...",
"created_at": "2016-04-12T08:35:54Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20388",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20388#issuecomment-294349",
"user": "https://github.com/mmasdeu"
}I have addressed Nils' two concerns above. About the "magma = Magma()" and the configuration parameters, I don't have a quick solution yet. There's a bunch of places in the schemes/ folder that import magma depending on the algorithm chosen. One easy way to fix this is to provide a get_magma_session() and set_magma_session() functions so that something like this would work:
sage: set_magma_session(Magma(server = 'remote', command = 'magma_custom'))
sage: function_using_magma(algorithm = 'magma')
This would only involve changing the calls that look like:
from sage.interfaces.magma import magma
magma.eval('complicated stuff')
to
from sage.interfaces.magma import get_magma_session
magma = get_magma_session()
magma.eval('complicated stuff')
Do you think that this is a good enough solution? If so, I would quickly implement it...
archive/issue_comments_294350.json:
{
"body": "<div id=\"comment:11\" align=\"right\">comment:11</div>\n\nReplying to [@mmasdeu](#comment%3A10):\n> I have addressed Nils' two concerns above. About the \"magma = Magma()\" and the configuration parameters, I don't have a quick solution yet. There's a bunch of places in the schemes/ folder that import magma depending on the algorithm chosen. One easy way to fix this is to provide a get_magma_session() and set_magma_session() functions so that something like this would work:\n> \n> ```\n> sage: set_magma_session(Magma(server = 'remote', command = 'magma_custom'))\n> sage: function_using_magma(algorithm = 'magma')\n> ```\n> \n> This would only involve changing the calls that look like:\n> \n> ```\n> from sage.interfaces.magma import magma\n> magma.eval('complicated stuff')\n> ```\n> to\n> \n> ```\n> from sage.interfaces.magma import get_magma_session\n> magma = get_magma_session()\n> magma.eval('complicated stuff')\n> ```\n> \n> Do you think that this is a good enough solution? If so, I would quickly implement it...\n\nI do not like it so much. It would involve a lot of changes in the schemes/ folder (that people have maybe already copy/paste in their personal code). And, more importantly, it would be different from any other interface in Sage. What about environment variable?\n\n```\n$ SAGE_MAGMA_COMMAND = \"module load magma/2.11.13; magma\"\n$ SAGE_MAGMA_SERVER = \"plafrim\"\n$ export SAGE_MAGMA_COMMAND SAGE_MAGMA_SERVER\n$ sage\n...\n```\n\nOne just needs in the constructor of `Magma` something like\n\n```\nclass Magma(Interface):\n def __init__(self, server=None, command=None, ...):\n import os\n if server is None:\n server = os.getenv('SAGE_MAGMA_SERVER')\n if command is None:\n command = os.getenv('SAGE_MAGMA_COMMAND')\n```\n\nA solution based on `init.sage` would not work since it is loaded after anything else.",
"created_at": "2016-04-12T11:50:29Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20388",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20388#issuecomment-294350",
"user": "https://github.com/videlec"
}Replying to @mmasdeu:
I have addressed Nils' two concerns above. About the "magma = Magma()" and the configuration parameters, I don't have a quick solution yet. There's a bunch of places in the schemes/ folder that import magma depending on the algorithm chosen. One easy way to fix this is to provide a get_magma_session() and set_magma_session() functions so that something like this would work:
sage: set_magma_session(Magma(server = 'remote', command = 'magma_custom')) sage: function_using_magma(algorithm = 'magma')This would only involve changing the calls that look like:
from sage.interfaces.magma import magma magma.eval('complicated stuff')to
from sage.interfaces.magma import get_magma_session magma = get_magma_session() magma.eval('complicated stuff')Do you think that this is a good enough solution? If so, I would quickly implement it...
I do not like it so much. It would involve a lot of changes in the schemes/ folder (that people have maybe already copy/paste in their personal code). And, more importantly, it would be different from any other interface in Sage. What about environment variable?
$ SAGE_MAGMA_COMMAND = "module load magma/2.11.13; magma"
$ SAGE_MAGMA_SERVER = "plafrim"
$ export SAGE_MAGMA_COMMAND SAGE_MAGMA_SERVER
$ sage
...
One just needs in the constructor of Magma something like
class Magma(Interface):
def __init__(self, server=None, command=None, ...):
import os
if server is None:
server = os.getenv('SAGE_MAGMA_SERVER')
if command is None:
command = os.getenv('SAGE_MAGMA_COMMAND')
A solution based on init.sage would not work since it is loaded after anything else.
archive/issue_comments_294351.json:
{
"body": "<div id=\"comment:12\" align=\"right\">comment:12</div>\n\n1) Some users may not be familiar with environment variables.\n2) I like to be able to write Sage scripts that work after you start sage. Using environment variables forces you to remember always to set them before you start your script, which can be annoying.\n3) The environment variable approach doesn't allow the user to compare the result of executing a function with two different Magma installations. Something like this would be desiderable.\n\n```\nsage: E = EllipticCurve('37a1')\nsage: set_magma_session(Magma(command = 'magma_2-18'))\nsage: E.analytic_rank(algorithm = 'magma')\n0\nsage: set_magma_session(Magma(command = 'magma_2-19'))\nsage: E.analytic_rank(algorithm = 'magma')\n0\n```\n4) I don't understand your first comment. If people has taken Sage code that uses Magma and now Sage adds more functionality, then they can either use the old functionality or adapt the code to the new (and better way), right?\n\nHere is what I propose: add the environment variables (which make sense the moment there is a magma = Magma() line in there...), but also change the functions using magma to use the get_magma_session() function. How reasonable is this?",
"created_at": "2016-04-12T13:16:41Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20388",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20388#issuecomment-294351",
"user": "https://github.com/mmasdeu"
}- Some users may not be familiar with environment variables.
- I like to be able to write Sage scripts that work after you start sage. Using environment variables forces you to remember always to set them before you start your script, which can be annoying.
- The environment variable approach doesn't allow the user to compare the result of executing a function with two different Magma installations. Something like this would be desiderable.
sage: E = EllipticCurve('37a1')
sage: set_magma_session(Magma(command = 'magma_2-18'))
sage: E.analytic_rank(algorithm = 'magma')
0
sage: set_magma_session(Magma(command = 'magma_2-19'))
sage: E.analytic_rank(algorithm = 'magma')
0
- I don't understand your first comment. If people has taken Sage code that uses Magma and now Sage adds more functionality, then they can either use the old functionality or adapt the code to the new (and better way), right?
Here is what I propose: add the environment variables (which make sense the moment there is a magma = Magma() line in there...), but also change the functions using magma to use the get_magma_session() function. How reasonable is this?
archive/issue_comments_294352.json:
{
"body": "<div id=\"comment:13\" align=\"right\">comment:13</div>\n\nReplying to [@mmasdeu](#comment%3A12): \n> Here is what I propose: add the environment variables (which make sense the moment there is a magma = Magma() line in there...), but also change the functions using magma to use the get_magma_session() function. How reasonable is this?\n\nYou are right that it is desirable to have a way to modify it from Sage itself. However, I would not populate the global namespace with `get_magma_session` unless the global `magma` is deprecated (which can be done using lazy imports).\n\nMoreover, it is very confusing to have all of: `magma`, `Magma`, `magma_free`, `magma_console`, `magma_version` in the global namespace... (not talking about `Magmas`). A cleanup is desirable.",
"created_at": "2016-04-12T14:50:18Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20388",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20388#issuecomment-294352",
"user": "https://github.com/videlec"
}Replying to @mmasdeu:
Here is what I propose: add the environment variables (which make sense the moment there is a magma = Magma() line in there...), but also change the functions using magma to use the get_magma_session() function. How reasonable is this?
You are right that it is desirable to have a way to modify it from Sage itself. However, I would not populate the global namespace with get_magma_session unless the global magma is deprecated (which can be done using lazy imports).
Moreover, it is very confusing to have all of: magma, Magma, magma_free, magma_console, magma_version in the global namespace... (not talking about Magmas). A cleanup is desirable.
archive/issue_comments_294353.json:
{
"body": "<div id=\"comment:14\" align=\"right\">comment:14</div>\n\nI think the `set_magma_session` is a rather heavy interface. It seems to me the appropriate place would be a method on `sage.interfaces.expect.Expect` along the lines of\n\n```\ndef set_server_and_command(self,server,command):\n if self._expect:\n raise RuntimeError(\"interface has already started\")\n self._server = server\n self.__command = command\n```\nIf there are other settings to change as well, it may be necessary to override this routine on Magma (and do the usual super() call thing)\n\nThe main thing is that an instantiated expect interface doesn't run a process on its own. Starting it is a separate operation (and in fact, during the lifetime of an expect object it may start and stop underlying processes repeatedly)\n\nThe cleaner solution would be to make new `Magma` instances for different server/command settings, but as we've seen a lot of code assumes that there is a single, global, magma instance for default use throughout the lifetime of a sage session.",
"created_at": "2016-04-12T15:49:53Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20388",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20388#issuecomment-294353",
"user": "https://github.com/nbruin"
}I think the set_magma_session is a rather heavy interface. It seems to me the appropriate place would be a method on sage.interfaces.expect.Expect along the lines of
def set_server_and_command(self,server,command):
if self._expect:
raise RuntimeError("interface has already started")
self._server = server
self.__command = command
If there are other settings to change as well, it may be necessary to override this routine on Magma (and do the usual super() call thing)
The main thing is that an instantiated expect interface doesn't run a process on its own. Starting it is a separate operation (and in fact, during the lifetime of an expect object it may start and stop underlying processes repeatedly)
The cleaner solution would be to make new Magma instances for different server/command settings, but as we've seen a lot of code assumes that there is a single, global, magma instance for default use throughout the lifetime of a sage session.
archive/issue_comments_294354.json:
{
"body": "<div id=\"comment:15\" align=\"right\">comment:15</div>\n\nReplying to [@nbruin](#comment%3A14):\n> I think the `set_magma_session` is a rather heavy interface. It seems to me the appropriate place would be a method on `sage.interfaces.expect.Expect` along the lines of\n> \n> ```\n> def set_server_and_command(self,server,command):\n> if self._expect:\n> raise RuntimeError(\"interface has already started\")\n> self._server = server\n> self.__command = command\n> ```\n\nI like better this approach. That way the server can even be configured in `init.sage`!\n\n> The cleaner solution would be to make new `Magma` instances for different server/command settings, but as we've seen a lot of code assumes that there is a single, global, magma instance for default use throughout the lifetime of a sage session. \n\nI would actually remove the `Magma` from the global namespace. That would not prevent anyone from testing different versions.",
"created_at": "2016-04-12T23:49:03Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20388",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20388#issuecomment-294354",
"user": "https://github.com/videlec"
}Replying to @nbruin:
I think the
set_magma_sessionis a rather heavy interface. It seems to me the appropriate place would be a method onsage.interfaces.expect.Expectalong the lines ofdef set_server_and_command(self,server,command): if self._expect: raise RuntimeError("interface has already started") self._server = server self.__command = command
I like better this approach. That way the server can even be configured in init.sage!
The cleaner solution would be to make new
Magmainstances for different server/command settings, but as we've seen a lot of code assumes that there is a single, global, magma instance for default use throughout the lifetime of a sage session.
I would actually remove the Magma from the global namespace. That would not prevent anyone from testing different versions.
archive/issue_comments_294355.json:
{
"body": "Changed branch from **[u/mmasdeu/20388](https://github.com/sagemath/sagetrac-mirror/tree/u/mmasdeu/20388)** to **[u/mmasdeu/20388-fix](https://github.com/sagemath/sagetrac-mirror/tree/u/mmasdeu/20388-fix)**",
"created_at": "2016-04-13T11:08:54Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20388",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20388#issuecomment-294355",
"user": "https://github.com/mmasdeu"
}Changed branch from u/mmasdeu/20388 to u/mmasdeu/20388-fix
archive/issue_comments_294356.json:
{
"body": "<div id=\"comment:16\"></div>\n\nNew commits:\n<table><tr><td><a href=\"https://github.com/sagemath/sagetrac-mirror/commit/d4c1650d466dd9ca5fa24769dc7450b9779ea080\"><code>d4c1650</code></a></td><td><code>Disallowed file transfers when using remote Magma. Added error checking</code></td></tr><tr><td><a href=\"https://github.com/sagemath/sagetrac-mirror/commit/d85b3900ade8c0826c34632fe3d1c4e17bb2c05f\"><code>d85b390</code></a></td><td><code>Added possibility of changing Magma defaults via environment variables.</code></td></tr><tr><td><a href=\"https://github.com/sagemath/sagetrac-mirror/commit/a8ee09a95a6bf7b80564dd644c7d82b609b790c7\"><code>a8ee09a</code></a></td><td><code>Added functionality in expect.py to set the server and command.</code></td></tr></table>\n",
"created_at": "2016-04-13T11:08:54Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20388",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20388#issuecomment-294356",
"user": "https://github.com/mmasdeu"
}New commits:
d4c1650 | Disallowed file transfers when using remote Magma. Added error checking |
d85b390 | Added possibility of changing Magma defaults via environment variables. |
a8ee09a | Added functionality in expect.py to set the server and command. |
archive/issue_comments_294357.json:
{
"body": "Changed commit from **[`da0c8a1`](https://github.com/sagemath/sagetrac-mirror/commit/da0c8a128c83b89f9cc6f42b3dfcdcac3f8fce24)** to **[`a8ee09a`](https://github.com/sagemath/sagetrac-mirror/commit/a8ee09a95a6bf7b80564dd644c7d82b609b790c7)**",
"created_at": "2016-04-13T11:08:54Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20388",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20388#issuecomment-294357",
"user": "https://github.com/mmasdeu"
}Changed commit from da0c8a1 to a8ee09a
archive/issue_comments_294358.json:
{
"body": "<div id=\"comment:17\" align=\"right\">comment:17</div>\n\nTested succesfully with `magma` and `matlab`! Great.\n\nThough, there is something wrong with `maple` (not sure where and how it might be related to the changes in this ticket)\n\n```\nsage: maple.set_server_and_command('calcul1')\nsage: maple('2+2') # hang out infinitely\n```\nAnd even after a Ctrl-C I do not have back the sage prompt (only `^CInterrupting Maple...`). After a second Ctrl-C I get the prompt back with the following traceback\n\n```\n...\n.../interfaces/maple.pyc in set(self, var, value)\n 619 \"\"\"\n 620 cmd = '%s:=%s:' % (var, value)\n--> 621 out = self.eval(cmd)\n 622 if out.find(\"error\") != -1:\n 623 raise TypeError(\"Error executing code in Maple\\nCODE:\\n\\t%s\\nMaple ERROR:\\n\\t%s\" % (cmd, out))\n\n.../interfaces/expect.pyc in eval(self, code, strip, synchronize, locals, allow_use_file, split_lines, **kwds)\n 1258 elif split_lines:\n 1259 return '\\n'.join([self._eval_line(L, allow_use_file=allow_use_file, **kwds)\n-> 1260 for L in code.split('\\n') if L != ''])\n 1261 else:\n 1262 return self._eval_line(code, allow_use_file=allow_use_file, **kwds)\n\n.../interfaces/maple.pyc in _eval_line(self, line, allow_use_file, wait_for_prompt, restart_if_needed)\n 571 with gc_disabled():\n 572 z = Expect._eval_line(self, line, allow_use_file=allow_use_file,\n--> 573 wait_for_prompt=wait_for_prompt).replace('\\\\\\n','').strip()\n 574 if z.lower().find(\"error\") != -1:\n 575 raise RuntimeError(\"An error occurred running a Maple command:\\nINPUT:\\n%s\\nOUTPUT:\\n%s\" % (line, z))\n...\n```\n(and `maple` works perfectly well on `calcul1` through `ssh`).",
"created_at": "2016-04-13T13:31:39Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20388",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20388#issuecomment-294358",
"user": "https://github.com/videlec"
}Tested succesfully with magma and matlab! Great.
Though, there is something wrong with maple (not sure where and how it might be related to the changes in this ticket)
sage: maple.set_server_and_command('calcul1')
sage: maple('2+2') # hang out infinitely
And even after a Ctrl-C I do not have back the sage prompt (only ^CInterrupting Maple...). After a second Ctrl-C I get the prompt back with the following traceback
...
.../interfaces/maple.pyc in set(self, var, value)
619 """
620 cmd = '%s:=%s:' % (var, value)
--> 621 out = self.eval(cmd)
622 if out.find("error") != -1:
623 raise TypeError("Error executing code in Maple\nCODE:\n\t%s\nMaple ERROR:\n\t%s" % (cmd, out))
.../interfaces/expect.pyc in eval(self, code, strip, synchronize, locals, allow_use_file, split_lines, **kwds)
1258 elif split_lines:
1259 return '\n'.join([self._eval_line(L, allow_use_file=allow_use_file, **kwds)
-> 1260 for L in code.split('\n') if L != ''])
1261 else:
1262 return self._eval_line(code, allow_use_file=allow_use_file, **kwds)
.../interfaces/maple.pyc in _eval_line(self, line, allow_use_file, wait_for_prompt, restart_if_needed)
571 with gc_disabled():
572 z = Expect._eval_line(self, line, allow_use_file=allow_use_file,
--> 573 wait_for_prompt=wait_for_prompt).replace('\\\n','').strip()
574 if z.lower().find("error") != -1:
575 raise RuntimeError("An error occurred running a Maple command:\nINPUT:\n%s\nOUTPUT:\n%s" % (line, z))
...
(and maple works perfectly well on calcul1 through ssh).
archive/issue_comments_294359.json:
{
"body": "<div id=\"comment:18\" align=\"right\">comment:18</div>\n\nMight be too much, but we could even move the environment part inside pexpect\n\n```\nserver = os.getenv('SAGE_{}_SERVER'.format(self.name().upper()))\n```\nWhat do you think?\n\nTo fit with coding conventions could change\n\n```\n(self,server = None,command = None, server_tmpdir = None, ulimit = None)\n-->\n(self, server=None, command=None, server_tmpdir=None, ulimit=None)\n```\nAnd add cross doctests between `set_server_and_command`, `server` and `command`.",
"created_at": "2016-04-13T13:33:41Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20388",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20388#issuecomment-294359",
"user": "https://github.com/videlec"
}Might be too much, but we could even move the environment part inside pexpect
server = os.getenv('SAGE_{}_SERVER'.format(self.name().upper()))
What do you think?
To fit with coding conventions could change
(self,server = None,command = None, server_tmpdir = None, ulimit = None)
-->
(self, server=None, command=None, server_tmpdir=None, ulimit=None)
And add cross doctests between set_server_and_command, server and command.
archive/issue_comments_294360.json:
{
"body": "<div id=\"comment:19\" align=\"right\">comment:19</div>\n\nAn annoying feature\n\n```\nsage: magma.set_<TAB>\nCreating list of all Magma intrinsics for use in tab completion.\n...\nScanning Magma types ...\n```",
"created_at": "2016-04-13T13:34:38Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20388",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20388#issuecomment-294360",
"user": "https://github.com/videlec"
}An annoying feature
sage: magma.set_<TAB>
Creating list of all Magma intrinsics for use in tab completion.
...
Scanning Magma types ...
archive/issue_comments_294361.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/8a28c1a382438c330b8a17297961605f4f3cd9c8\"><code>8a28c1a</code></a></td><td><code>Added examples. Moved environment variables to expect.</code></td></tr></table>\n",
"created_at": "2016-04-13T20:08:10Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20388",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20388#issuecomment-294361",
"user": "https://github.com/sagetrac-git"
}Branch pushed to git repo; I updated commit sha1. New commits:
8a28c1a | Added examples. Moved environment variables to expect. |
archive/issue_comments_294362.json:
{
"body": "Changed commit from **[`a8ee09a`](https://github.com/sagemath/sagetrac-mirror/commit/a8ee09a95a6bf7b80564dd644c7d82b609b790c7)** to **[`8a28c1a`](https://github.com/sagemath/sagetrac-mirror/commit/8a28c1a382438c330b8a17297961605f4f3cd9c8)**",
"created_at": "2016-04-13T20:08:10Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20388",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20388#issuecomment-294362",
"user": "https://github.com/sagetrac-git"
}Changed commit from a8ee09a to 8a28c1a
archive/issue_comments_294363.json:
{
"body": "<div id=\"comment:21\" align=\"right\">comment:21</div>\n\nReplying to [@videlec](#comment%3A19):\n> An annoying feature\n> \n> ```\n> sage: magma.set_<TAB>\n> Creating list of all Magma intrinsics for use in tab completion.\n> ...\n> Scanning Magma types ...\n> ```\n\nThis only happens the first time you use a server/tmpdir (as long as tmpdir is not cleaned). It is part of the fun to be able to use the discovery feature of python with Magma! So this should be seen as a nice feature, and it was done in your computer the first time you used the magma session as well...",
"created_at": "2016-04-13T20:10:26Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20388",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20388#issuecomment-294363",
"user": "https://github.com/mmasdeu"
}Replying to @videlec:
An annoying feature
sage: magma.set_<TAB> Creating list of all Magma intrinsics for use in tab completion. ... Scanning Magma types ...
This only happens the first time you use a server/tmpdir (as long as tmpdir is not cleaned). It is part of the fun to be able to use the discovery feature of python with Magma! So this should be seen as a nice feature, and it was done in your computer the first time you used the magma session as well...
archive/issue_comments_294364.json:
{
"body": "<div id=\"comment:22\" align=\"right\">comment:22</div>\n\nReplying to [@videlec](#comment%3A17):\n\n> Though, there is something wrong with `maple` (not sure where and how it might be related to the changes in this ticket)\n\nI have no easy way to debug this. Maybe open a new ticket?",
"created_at": "2016-04-13T20:12:36Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20388",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20388#issuecomment-294364",
"user": "https://github.com/mmasdeu"
}Replying to @videlec:
Though, there is something wrong with
maple(not sure where and how it might be related to the changes in this ticket)
I have no easy way to debug this. Maybe open a new ticket?
archive/issue_comments_294365.json:
{
"body": "<div id=\"comment:23\" align=\"right\">comment:23</div>\n\nDone!\n\nReplying to [@videlec](#comment%3A18):\n> Might be too much, but we could even move the environment part inside pexpect\n> \n> ```\n> server = os.getenv('SAGE_{}_SERVER'.format(self.name().upper()))\n> ```\n> What do you think?\n> \n> To fit with coding conventions could change\n> \n> ```\n> (self,server = None,command = None, server_tmpdir = None, ulimit = None)\n> -->\n> (self, server=None, command=None, server_tmpdir=None, ulimit=None)\n> ```\n> And add cross doctests between `set_server_and_command`, `server` and `command`.",
"created_at": "2016-04-13T20:13:07Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20388",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20388#issuecomment-294365",
"user": "https://github.com/mmasdeu"
}Done!
Replying to @videlec:
Might be too much, but we could even move the environment part inside pexpect
server = os.getenv('SAGE_{}_SERVER'.format(self.name().upper()))What do you think?
To fit with coding conventions could change
(self,server = None,command = None, server_tmpdir = None, ulimit = None) --> (self, server=None, command=None, server_tmpdir=None, ulimit=None)And add cross doctests between
set_server_and_command,serverandcommand.
archive/issue_comments_294366.json:
{
"body": "<div id=\"comment:24\" align=\"right\">comment:24</div>\n\nReplying to [@mmasdeu](#comment%3A22):\n> Replying to [@videlec](#comment%3A17):\n> \n> > Though, there is something wrong with `maple` (not sure where and how it might be related to the changes in this ticket)\n> \n> \n> I have no easy way to debug this. Maybe open a new ticket?\n\nIt is now working (with and without your branch)... strange.",
"created_at": "2016-04-13T20:29:43Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20388",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20388#issuecomment-294366",
"user": "https://github.com/videlec"
}Replying to @mmasdeu:
Replying to @videlec:
Though, there is something wrong with
maple(not sure where and how it might be related to the changes in this ticket)I have no easy way to debug this. Maybe open a new ticket?
It is now working (with and without your branch)... strange.
archive/issue_comments_294367.json:
{
"body": "<div id=\"comment:25\" align=\"right\">comment:25</div>\n\nWe have a strange message before the Sage prompt if we do not specify the tmp repository\n\n```\n$ export SAGE_MAPLE_SERVER='calcul1'\n$ sage\nSageMath version ...\nNo remote temporary directory (option server_tmpdir) specified,\nusing /tmp/ on calcul1\nsage: \n```\nDo you think this is ok? Everything is fine with\n\n```\n$ export SAGE_MAPLE_SERVER='calcul1'\n$ export SAGE_MAPLE_TMPDIR='/tmp'\n$ sage\n```",
"created_at": "2016-04-13T20:37:15Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20388",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20388#issuecomment-294367",
"user": "https://github.com/videlec"
}We have a strange message before the Sage prompt if we do not specify the tmp repository
$ export SAGE_MAPLE_SERVER='calcul1'
$ sage
SageMath version ...
No remote temporary directory (option server_tmpdir) specified,
using /tmp/ on calcul1
sage:
Do you think this is ok? Everything is fine with
$ export SAGE_MAPLE_SERVER='calcul1'
$ export SAGE_MAPLE_TMPDIR='/tmp'
$ sage
archive/issue_comments_294368.json:
{
"body": "<div id=\"comment:26\" align=\"right\">comment:26</div>\n\nYes, if you don't specify the tmpdir, then this message gets printed (this was already there when I looked at the code). You can specify the tmpdir either in the environment (as you do) or from within the set_server_and_command method.",
"created_at": "2016-04-13T20:39:27Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20388",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20388#issuecomment-294368",
"user": "https://github.com/mmasdeu"
}Yes, if you don't specify the tmpdir, then this message gets printed (this was already there when I looked at the code). You can specify the tmpdir either in the environment (as you do) or from within the set_server_and_command method.
archive/issue_comments_294369.json:
{
"body": "<div id=\"comment:27\" align=\"right\">comment:27</div>\n\nReplying to [@mmasdeu](#comment%3A21):\n> Replying to [@videlec](#comment%3A19):\n> > An annoying feature\n> > \n> > ```\n> > sage: magma.set_<TAB>\n> > Creating list of all Magma intrinsics for use in tab completion.\n> > ...\n> > Scanning Magma types ...\n> > ```\n> \n> \n> This only happens the first time you use a server/tmpdir (as long as tmpdir is not cleaned). It is part of the fun to be able to use the discovery feature of python with Magma! So this should be seen as a nice feature, and it was done in your computer the first time you used the magma session as well...\n\nNot exactly, it is done the first time you use tab completion with magma. And the message says that it is saved **locally** in `.sage//magma_intrinsic_cache.sobj` (with two ugly slashes). Might be better to make it parameters dependent because two magma versions might have different namespaces, no? (anyway not for this ticket)",
"created_at": "2016-04-13T20:40:14Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20388",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20388#issuecomment-294369",
"user": "https://github.com/videlec"
}Replying to @mmasdeu:
Replying to @videlec:
An annoying feature
sage: magma.set_<TAB> Creating list of all Magma intrinsics for use in tab completion. ... Scanning Magma types ...This only happens the first time you use a server/tmpdir (as long as tmpdir is not cleaned). It is part of the fun to be able to use the discovery feature of python with Magma! So this should be seen as a nice feature, and it was done in your computer the first time you used the magma session as well...
Not exactly, it is done the first time you use tab completion with magma. And the message says that it is saved locally in .sage//magma_intrinsic_cache.sobj (with two ugly slashes). Might be better to make it parameters dependent because two magma versions might have different namespaces, no? (anyway not for this ticket)
archive/issue_comments_294370.json:
{
"body": "<div id=\"comment:28\" align=\"right\">comment:28</div>\n\nSided remark: there is a `.format()` trick to deal with curly brackets. Double brackets are transformed into one\n\n```\nsage: 'SAGE_{{{{}}}}_{{}}_{}'.format('one').format('two').format('three')\n'SAGE_three_two_one'\nsage: '{{{}}}'.format(1)\n'{1}'\n```",
"created_at": "2016-04-13T20:45:32Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20388",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20388#issuecomment-294370",
"user": "https://github.com/videlec"
}Sided remark: there is a .format() trick to deal with curly brackets. Double brackets are transformed into one
sage: 'SAGE_{{{{}}}}_{{}}_{}'.format('one').format('two').format('three')
'SAGE_three_two_one'
sage: '{{{}}}'.format(1)
'{1}'
archive/issue_comments_294371.json:
{
"body": "<div id=\"comment:29\" align=\"right\">comment:29</div>\n\nAs you moved the code of `magma_version` into the class, you should deprecate it (since it is in the global namespace).",
"created_at": "2016-04-13T20:53:03Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20388",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20388#issuecomment-294371",
"user": "https://github.com/videlec"
}As you moved the code of magma_version into the class, you should deprecate it (since it is in the global namespace).
archive/issue_events_285079.json:
{
"actor": "https://github.com/videlec",
"created_at": "2016-04-13T20:59:37Z",
"event": "unlabeled",
"issue": "https://github.com/sagemath/sage/issues/20388",
"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/20388#event-285079"
}archive/issue_events_285080.json:
{
"actor": "https://github.com/videlec",
"created_at": "2016-04-13T20:59:37Z",
"event": "labeled",
"issue": "https://github.com/sagemath/sage/issues/20388",
"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/20388#event-285080"
}archive/issue_comments_294372.json:
{
"body": "<div id=\"comment:30\" align=\"right\">comment:30</div>\n\nIn the `Magma` constructor there is the default option `command = 'magma'` which prevents using the environment variable `SAGE_MAGMA_COMMAND`.",
"created_at": "2016-04-13T20:59:37Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20388",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20388#issuecomment-294372",
"user": "https://github.com/videlec"
}In the Magma constructor there is the default option command = 'magma' which prevents using the environment variable SAGE_MAGMA_COMMAND.
archive/issue_comments_294373.json:
{
"body": "<div id=\"comment:31\" align=\"right\">comment:31</div>\n\nSetting the default to `None` is incompatible with\n\n```\n if not user_config:\n command += ' -n'\n```\nwhich results in\n\n```\nTypeError: unsupported operand type(s) for +=: 'NoneType' and 'str'\n```",
"created_at": "2016-04-13T21:01:31Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20388",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20388#issuecomment-294373",
"user": "https://github.com/videlec"
}Setting the default to None is incompatible with
if not user_config:
command += ' -n'
which results in
TypeError: unsupported operand type(s) for +=: 'NoneType' and 'str'
archive/issue_comments_294374.json:
{
"body": "Changed commit from **[`8a28c1a`](https://github.com/sagemath/sagetrac-mirror/commit/8a28c1a382438c330b8a17297961605f4f3cd9c8)** to **[`3197787`](https://github.com/sagemath/sagetrac-mirror/commit/31977871ebc1d25677951f8cbbe6dee3105c9868)**",
"created_at": "2016-04-14T07:27:41Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20388",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20388#issuecomment-294374",
"user": "https://github.com/sagetrac-git"
}Changed commit from 8a28c1a to 3197787
archive/issue_comments_294375.json:
{
"body": "<div id=\"comment:32\"></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/31977871ebc1d25677951f8cbbe6dee3105c9868\"><code>3197787</code></a></td><td><code>Fixed user_config and environment variable clash.</code></td></tr></table>\n",
"created_at": "2016-04-14T07:27:41Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20388",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20388#issuecomment-294375",
"user": "https://github.com/sagetrac-git"
}Branch pushed to git repo; I updated commit sha1. New commits:
3197787 | Fixed user_config and environment variable clash. |
archive/issue_comments_294376.json:
{
"body": "<div id=\"comment:33\" align=\"right\">comment:33</div>\n\nI fixed it by (also) reading the environment variable in magma.py. Each interface can do something similar if needed.",
"created_at": "2016-04-14T07:28:17Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20388",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20388#issuecomment-294376",
"user": "https://github.com/mmasdeu"
}I fixed it by (also) reading the environment variable in magma.py. Each interface can do something similar if needed.
archive/issue_comments_294377.json:
{
"body": "<div id=\"comment:34\" align=\"right\">comment:34</div>\n\nReplying to [@videlec](#comment%3A27):\n> Not exactly, it is done the first time you use tab completion with magma. And the message says that it is saved **locally** in `.sage//magma_intrinsic_cache.sobj` (with two ugly slashes). Might be better to make it parameters dependent because two magma versions might have different namespaces, no? (anyway not for this ticket)\n\nNo, that is the best place to save it because that's one of the few (hopefully persistent) places where you might expect to have write access. Two magma versions tend to have only subtly different namespaces by default, so the enormous cost of trying to differentiate is probably not offset by having slightly more accurate tab completion.",
"created_at": "2016-04-14T07:54:54Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20388",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20388#issuecomment-294377",
"user": "https://github.com/nbruin"
}Replying to @videlec:
Not exactly, it is done the first time you use tab completion with magma. And the message says that it is saved locally in
.sage//magma_intrinsic_cache.sobj(with two ugly slashes). Might be better to make it parameters dependent because two magma versions might have different namespaces, no? (anyway not for this ticket)
No, that is the best place to save it because that's one of the few (hopefully persistent) places where you might expect to have write access. Two magma versions tend to have only subtly different namespaces by default, so the enormous cost of trying to differentiate is probably not offset by having slightly more accurate tab completion.
archive/issue_events_285081.json:
{
"actor": "https://github.com/mmasdeu",
"created_at": "2016-04-14T09:58:00Z",
"event": "unlabeled",
"issue": "https://github.com/sagemath/sage/issues/20388",
"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/20388#event-285081"
}archive/issue_events_285082.json:
{
"actor": "https://github.com/mmasdeu",
"created_at": "2016-04-14T09:58:00Z",
"event": "labeled",
"issue": "https://github.com/sagemath/sage/issues/20388",
"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/20388#event-285082"
}archive/issue_comments_294378.json:
{
"body": "<div id=\"comment:36\"></div>\n\nBranch pushed to git repo; I updated commit sha1. New commits:\n<table><tr><td><a href=\"https://github.com/sagemath/sagetrac-mirror/commit/ce26d46998e1c1be16babab4225dc0f9e1dac281\"><code>ce26d46</code></a></td><td><code>Removed command_args parameter from one place.</code></td></tr></table>\n",
"created_at": "2016-04-14T10:07:58Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20388",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20388#issuecomment-294378",
"user": "https://github.com/sagetrac-git"
}Branch pushed to git repo; I updated commit sha1. New commits:
ce26d46 | Removed command_args parameter from one place. |
archive/issue_comments_294379.json:
{
"body": "Changed commit from **[`3197787`](https://github.com/sagemath/sagetrac-mirror/commit/31977871ebc1d25677951f8cbbe6dee3105c9868)** to **[`ce26d46`](https://github.com/sagemath/sagetrac-mirror/commit/ce26d46998e1c1be16babab4225dc0f9e1dac281)**",
"created_at": "2016-04-14T10:07:58Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20388",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20388#issuecomment-294379",
"user": "https://github.com/sagetrac-git"
}Changed commit from 3197787 to ce26d46
archive/issue_comments_294380.json:
{
"body": "<div id=\"comment:37\" align=\"right\">comment:37</div>\n\nReplying to [@nbruin](#comment%3A34):\n> Replying to [@videlec](#comment%3A27):\n> > Not exactly, it is done the first time you use tab completion with magma. And the message says that it is saved **locally** in `.sage//magma_intrinsic_cache.sobj` (with two ugly slashes). Might be better to make it parameters dependent because two magma versions might have different namespaces, no? (anyway not for this ticket)\n> \n> \n> No, that is the best place to save it because that's one of the few (hopefully persistent) places where you might expect to have write access. Two magma versions tend to have only subtly different namespaces by default, so the enormous cost of trying to differentiate is probably not offset by having slightly more accurate tab completion.\n\nI was not complaining about the location, just about potential version clashes. However, this seems to be completely broken (at least with magma 2.11.13 on my computer).",
"created_at": "2016-04-14T12:49:42Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20388",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20388#issuecomment-294380",
"user": "https://github.com/videlec"
}Replying to @nbruin:
Replying to @videlec:
Not exactly, it is done the first time you use tab completion with magma. And the message says that it is saved locally in
.sage//magma_intrinsic_cache.sobj(with two ugly slashes). Might be better to make it parameters dependent because two magma versions might have different namespaces, no? (anyway not for this ticket)No, that is the best place to save it because that's one of the few (hopefully persistent) places where you might expect to have write access. Two magma versions tend to have only subtly different namespaces by default, so the enormous cost of trying to differentiate is probably not offset by having slightly more accurate tab completion.
I was not complaining about the location, just about potential version clashes. However, this seems to be completely broken (at least with magma 2.11.13 on my computer).
archive/issue_comments_294381.json:
{
"body": "<div id=\"comment:38\" align=\"right\">comment:38</div>\n\nMarc, you did not address [comment:29], see [Deprecation in the developer guide](http://doc.sagemath.org/html/en/developer/coding_in_python.html#deprecation).",
"created_at": "2016-04-14T14:04:05Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20388",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20388#issuecomment-294381",
"user": "https://github.com/videlec"
}Marc, you did not address [comment:29], see Deprecation in the developer guide.
archive/issue_events_285083.json:
{
"actor": "https://github.com/videlec",
"created_at": "2016-04-14T14:04:05Z",
"event": "unlabeled",
"issue": "https://github.com/sagemath/sage/issues/20388",
"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/20388#event-285083"
}archive/issue_events_285084.json:
{
"actor": "https://github.com/videlec",
"created_at": "2016-04-14T14:04:05Z",
"event": "labeled",
"issue": "https://github.com/sagemath/sage/issues/20388",
"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/20388#event-285084"
}archive/issue_comments_294382.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/c079a14424e9a1ea77179c658284b867b8401ef8\"><code>c079a14</code></a></td><td><code>Deprecated and removed from global namesplace the function magma_version.</code></td></tr></table>\n",
"created_at": "2016-04-14T20:26:21Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20388",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20388#issuecomment-294382",
"user": "https://github.com/sagetrac-git"
}Branch pushed to git repo; I updated commit sha1. New commits:
c079a14 | Deprecated and removed from global namesplace the function magma_version. |
archive/issue_comments_294383.json:
{
"body": "Changed commit from **[`ce26d46`](https://github.com/sagemath/sagetrac-mirror/commit/ce26d46998e1c1be16babab4225dc0f9e1dac281)** to **[`c079a14`](https://github.com/sagemath/sagetrac-mirror/commit/c079a14424e9a1ea77179c658284b867b8401ef8)**",
"created_at": "2016-04-14T20:26:21Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20388",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20388#issuecomment-294383",
"user": "https://github.com/sagetrac-git"
}Changed commit from ce26d46 to c079a14
archive/issue_comments_294384.json:
{
"body": "<div id=\"comment:40\" align=\"right\">comment:40</div>\n\nI have deprecated the function magma_version. When calling it, you get now an explanatory message. I get a ton of other deprecation warnings which I think are not my fault, so I hope that they don't delay the review of this ticket!",
"created_at": "2016-04-14T20:28:47Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20388",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20388#issuecomment-294384",
"user": "https://github.com/mmasdeu"
}I have deprecated the function magma_version. When calling it, you get now an explanatory message. I get a ton of other deprecation warnings which I think are not my fault, so I hope that they don't delay the review of this ticket!
archive/issue_events_285085.json:
{
"actor": "https://github.com/mmasdeu",
"created_at": "2016-04-14T20:28:47Z",
"event": "unlabeled",
"issue": "https://github.com/sagemath/sage/issues/20388",
"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/20388#event-285085"
}archive/issue_events_285086.json:
{
"actor": "https://github.com/mmasdeu",
"created_at": "2016-04-14T20:28:47Z",
"event": "labeled",
"issue": "https://github.com/sagemath/sage/issues/20388",
"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/20388#event-285086"
}archive/issue_comments_294385.json:
{
"body": "<div id=\"comment:41\" align=\"right\">comment:41</div>\n\nReplying to [@mmasdeu](#comment%3A40):\n> I have deprecated the function magma_version. When calling it, you get now an explanatory message. I get a ton of other deprecation warnings which I think are not my fault, so I hope that they don't delay the review of this ticket!\n\nNope. There is another ticket which actually tracks the issue. This is a problem with the way Sage uses Ipython.",
"created_at": "2016-04-14T20:48:08Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20388",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20388#issuecomment-294385",
"user": "https://github.com/videlec"
}Replying to @mmasdeu:
I have deprecated the function magma_version. When calling it, you get now an explanatory message. I get a ton of other deprecation warnings which I think are not my fault, so I hope that they don't delay the review of this ticket!
Nope. There is another ticket which actually tracks the issue. This is a problem with the way Sage uses Ipython.
archive/issue_comments_294386.json:
{
"body": "<div id=\"comment:42\" align=\"right\">comment:42</div>\n\nTechnically, you deprecated the import in the global namespace but not the function in the module. In other words, this does not raise a warning as it should\n\n```\nsage: sage.interfaces.magma.magma_version()\n```\n(because in the long run, we should just get rid of `magma_version`). Would be better to simply deprecate `magma_version` and not modify `all.py` using\n\n```\ndef magma_version():\n from sage.misc.superseded import deprecation\n deprecation(20388, 'whatever')\n return magma.version()\n```\n\nOther than that, everything looks good to me. Nils?",
"created_at": "2016-04-14T20:48:15Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20388",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20388#issuecomment-294386",
"user": "https://github.com/videlec"
}Technically, you deprecated the import in the global namespace but not the function in the module. In other words, this does not raise a warning as it should
sage: sage.interfaces.magma.magma_version()
(because in the long run, we should just get rid of magma_version). Would be better to simply deprecate magma_version and not modify all.py using
def magma_version():
from sage.misc.superseded import deprecation
deprecation(20388, 'whatever')
return magma.version()
Other than that, everything looks good to me. Nils?
archive/issue_comments_294387.json:
{
"body": "<div id=\"comment:43\"></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/6c7236e3c3934fbd8b0a4b473acaa5b2243cd856\"><code>6c7236e</code></a></td><td><code>Revert \"Deprecated and removed from global namesplace the function magma_version.\"</code></td></tr><tr><td><a href=\"https://github.com/sagemath/sagetrac-mirror/commit/f4996488101ac3ce85550fb14b2eec08185408a6\"><code>f499648</code></a></td><td><code>Deprecated magma_version instead of its importing.</code></td></tr></table>\n",
"created_at": "2016-04-14T21:08:17Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20388",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20388#issuecomment-294387",
"user": "https://github.com/sagetrac-git"
}Branch pushed to git repo; I updated commit sha1. New commits:
6c7236e | Revert "Deprecated and removed from global namesplace the function magma_version." |
f499648 | Deprecated magma_version instead of its importing. |
archive/issue_comments_294388.json:
{
"body": "Changed commit from **[`c079a14`](https://github.com/sagemath/sagetrac-mirror/commit/c079a14424e9a1ea77179c658284b867b8401ef8)** to **[`f499648`](https://github.com/sagemath/sagetrac-mirror/commit/f4996488101ac3ce85550fb14b2eec08185408a6)**",
"created_at": "2016-04-14T21:08:17Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20388",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20388#issuecomment-294388",
"user": "https://github.com/sagetrac-git"
}Changed commit from c079a14 to f499648
archive/issue_comments_294389.json:
{
"body": "<div id=\"comment:44\" align=\"right\">comment:44</div>\n\nThis good for me, only needs Nils agreement. Thanks for the fix. Possible follow-up: #13885",
"created_at": "2016-04-14T21:11:54Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20388",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20388#issuecomment-294389",
"user": "https://github.com/videlec"
}This good for me, only needs Nils agreement. Thanks for the fix. Possible follow-up: #13885
archive/issue_comments_294390.json:
{
"body": "<div id=\"comment:45\" align=\"right\">comment:45</div>\n\nI don't have an axe to grind on this. My general impression is that the proposal here should improve the status quo.",
"created_at": "2016-04-15T05:46:53Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20388",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20388#issuecomment-294390",
"user": "https://github.com/nbruin"
}I don't have an axe to grind on this. My general impression is that the proposal here should improve the status quo.
archive/issue_comments_294391.json:
{
"body": "Reviewer: **Nils Bruin, Vincent Delecroix**",
"created_at": "2016-04-15T12:54:36Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20388",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20388#issuecomment-294391",
"user": "https://github.com/videlec"
}Reviewer: Nils Bruin, Vincent Delecroix
archive/issue_events_285087.json:
{
"actor": "https://github.com/videlec",
"created_at": "2016-04-15T12:54:36Z",
"event": "unlabeled",
"issue": "https://github.com/sagemath/sage/issues/20388",
"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/20388#event-285087"
}archive/issue_events_285088.json:
{
"actor": "https://github.com/videlec",
"created_at": "2016-04-15T12:54:36Z",
"event": "labeled",
"issue": "https://github.com/sagemath/sage/issues/20388",
"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/20388#event-285088"
}archive/issue_events_285089.json:
{
"actor": "https://github.com/vbraun",
"created_at": "2016-04-16T10:25:59Z",
"event": "unlabeled",
"issue": "https://github.com/sagemath/sage/issues/20388",
"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/20388#event-285089"
}archive/issue_events_285090.json:
{
"actor": "https://github.com/vbraun",
"commit_id": "4b72d69a7856c910e8528c818d43b287a8ad584e",
"commit_repository": "https://github.com/sagemath/sage",
"created_at": "2016-04-16T10:25:59Z",
"event": "closed",
"issue": "https://github.com/sagemath/sage/issues/20388",
"type": "issue_event",
"url": "https://github.com/sagemath/sage/issues/20388#event-285090"
}archive/issue_comments_294392.json:
{
"body": "Changed branch from **[u/mmasdeu/20388-fix](https://github.com/sagemath/sagetrac-mirror/tree/u/mmasdeu/20388-fix)** to **[`f499648`](https://github.com/sagemath/sagetrac-mirror/commit/f4996488101ac3ce85550fb14b2eec08185408a6)**",
"created_at": "2016-04-16T10:25:59Z",
"formatter": "markdown",
"issue": "https://github.com/sagemath/sage/issues/20388",
"type": "issue_comment",
"url": "https://github.com/sagemath/sage/issues/20388#issuecomment-294392",
"user": "https://github.com/vbraun"
}Changed branch from u/mmasdeu/20388-fix to f499648