Opened 9 years ago
Closed 8 years ago
#16640 closed defect (fixed)
Graphics3d.show abuses graphics_filename
Reported by: | gagern | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-6.4 |
Component: | graphics | Keywords: | |
Cc: | Merged in: | ||
Authors: | Martin von Gagern, Jeroen Demeyer | Reviewers: | Jeroen Demeyer, Karl-Dieter Crisman |
Report Upstream: | N/A | Work issues: | |
Branch: | 034fa58 (Commits, GitHub, GitLab) | Commit: | 034fa58b0051be5db6942a76c06010e15a3d664b |
Dependencies: | Stopgaps: |
Description
Code in sage/plot/plot3d/base.pyx in the method Graphics3d.show()
makes invalid use of graphics_filename
: it generates a name (like sage0.png
), then strips the extension and appends various other extensions during the course of the method. But graphics_filename
makes no guarantees that these other file names will be unused. In fact chances are pretty good that these files already exist from a different run of that method inside the same cell. So we get clashes there. Perhaps if things change in other places we might even get a security issue due to a race condition here.
I can think of two possible approaches: either call graphics_filename
in several places, once for every file name we need to generate. Or create a temporary directory and write all files inside that with fixed names. The former will likely be easier to handle for notebook, but may make the code harder to read. Particularly those cases where some tool will use one file name to derive another file name might be a real pain, like when the applet size is encoded in the file name, or the magic aroud the archive_name
for jmol. Will need some good interactive testing to make sure that everything still works after these changes.
Change History (64)
comment:1 Changed 9 years ago by
Branch: | → u/gagern/ticket/16640 |
---|---|
Created: | Jul 10, 2014, 6:59:14 AM → Jul 10, 2014, 6:59:14 AM |
Modified: | Jul 10, 2014, 6:59:14 AM → Jul 10, 2014, 6:59:14 AM |
comment:2 Changed 9 years ago by
Authors: | → Martin von Gagern |
---|---|
Commit: | → 9dfe1337683a6d99118b658fad39d55a0110692a |
Dependencies: | → #16645 |
Status: | new → needs_review |
comment:4 Changed 8 years ago by
Milestone: | sage-6.3 → sage-6.4 |
---|
comment:5 Changed 8 years ago by
Commit: | 9dfe1337683a6d99118b658fad39d55a0110692a → cf63c2dd372caa602e67397e871ac65c164a2cba |
---|
comment:6 Changed 8 years ago by
Dependencies: | #16645 |
---|---|
Modified: | Sep 5, 2014, 7:19:46 AM → Sep 5, 2014, 7:19:46 AM |
comment:7 Changed 8 years ago by
Commit: | cf63c2dd372caa602e67397e871ac65c164a2cba → 84a2f30d7d494ece9772605a86b85d3b90742fab |
---|
comment:8 Changed 8 years ago by
Branch: | u/gagern/ticket/16640 → u/jdemeyer/ticket/16640 |
---|---|
Modified: | Sep 5, 2014, 7:26:24 AM → Sep 5, 2014, 7:26:24 AM |
comment:9 Changed 8 years ago by
Commit: | 84a2f30d7d494ece9772605a86b85d3b90742fab → 49ce20b63e2dc39d611f453f76077620f4faccfb |
---|
comment:10 Changed 8 years ago by
Reviewers: | → Jeroen Demeyer |
---|
Merged with 6.4.beta4 and added some fixes. Please review.
comment:11 Changed 8 years ago by
Commit: | 49ce20b63e2dc39d611f453f76077620f4faccfb → 463eb63585d45e209938fcd8b8dd91d1a488d2e4 |
---|
Branch pushed to git repo; I updated commit sha1. New commits:
463eb63 | Rendering canvas3d in all doctests is too slow
|
comment:12 Changed 8 years ago by
Commit: | 463eb63585d45e209938fcd8b8dd91d1a488d2e4 → 711127f7cf89f6e75d7f2e970c4a58ec51ec03c5 |
---|
Branch pushed to git repo; I updated commit sha1. New commits:
711127f | Merge remote-tracking branch 'origin/develop' into ticket/16640
|
comment:13 follow-up: 14 Changed 8 years ago by
Hi Jeroen: Your first two commits are fine, as is the first commit of Martin's. (Currently looking at his main, second one.) How much slower did the doctests become without your last commit? (Martin, if you see this, what was your thinking on changing the doctest mode?)
comment:14 follow-up: 17 Changed 8 years ago by
Hi, sorry I didn't respond to the commits by Jeroen any sooner.
Replying to kcrisman:
Hi Jeroen: Your first two commits are fine,
I agree. Thanks a lot!
(Martin, if you see this, what was your thinking on changing the doctest mode?)
My main reason for modifying that line at all was the fact that canvas3d only works in embedded mode, so having it available otherwise might confuse users. My reson for including it in doctests is simply because all the other possible viewers are active in doctest mode as well. There is a doctest specifically for the canvas3d case, but if I restrict canvas3d to embedded mode only, that one would fail. So staying in line with all the other tests seemed the reasonable thing to do.
If things really are too slow, I'd change that to
if viewer == 'canvas3d' and (DOCTEST_MODE or EMBEDDED_MODE):
so we can have that one explicit doctest nevertheless. But first I'd like to quantify that effect on test execution time, and right now I have a build problem.
comment:15 Changed 8 years ago by
Always doing canvas3d
is really too slow, I remember one file(!) taking over 1000s to doctest.
comment:16 Changed 8 years ago by
Commit: | 711127f7cf89f6e75d7f2e970c4a58ec51ec03c5 → bd1479d38fe1adb42755ab92499ff8c1ba35592d |
---|
Branch pushed to git repo; I updated commit sha1. New commits:
bd1479d | Make viewer="canvas3d" an error outside the Notebook
|
comment:17 follow-up: 18 Changed 8 years ago by
Replying to gagern:
My main reason for modifying that line at all was the fact that canvas3d only works in embedded mode, so having it available otherwise might confuse users.
Fixed by raising a suitable exception.
comment:18 Changed 8 years ago by
My main reason for modifying that line at all was the fact that canvas3d only works in embedded mode, so having it available otherwise might confuse users.
Fixed by raising a suitable exception.
That was something we needed a while ago anyway! Thanks.
comment:19 follow-ups: 23 29 Changed 8 years ago by
Here is something minor but still needing attention. The documentation says
- ``filename`` -- string (default: a temp file); file to save the image to
but of course this is now only going to be true in the command line. Which probably makes sense. But this should be noted. Similarly,
# Tachyon resolution options
should probably just add a quick note that "mode defaults" also are being set in the next few lines.
Interestingly,
# Temporary hack: encode the desired applet size in the end of the filename: # (This will be removed once we have dynamic resizing of applets in the browser.)
and the following code could in principle be removed, as the update does indeed have dynamic resizing! But I think let's leave that be for somewhere else; I don't want to mess with the already messy naming of jmol stuff...
Finally, I don't understand this comment - I'm sorry if I am just thinking inside the box:
# when testing this, modify graphics_filename so that it # returns files which don't automatically share a base name, # e.g.: "import random; i = random.randint(0, 10000)"
Was this just a "note to self" or do you mean that, as a tester, I should actually change graphics_filename
instead of letting it just keep adding one to base names, or ... ? I figured the whole point of this change was to test where they did share a base name, but again I may be thinking too conventionally.
comment:20 Changed 8 years ago by
So far it seems fine, though I rediscovered and diagnosed #11275 along the way.
comment:21 follow-ups: 22 48 Changed 8 years ago by
Status: | needs_review → needs_work |
---|
The following in the command line don't seem to create any files (I mean visible ones in my directory). Maybe they aren't supposed to?
sage: var('y') y sage: plot3d(x^2+y^3,(x,-1,1),(y,-1,1),filename='foo') sage: plot3d(x^2+y^3,(x,-1,1),(y,-1,1),filename='foo',viewer='tachyon')
Note that the second thing pops up a jmol viewer. I can't figure out if this ticket introduced it, though it seems to predate that, based on my experimentation.
Also, with the current documentation, there is no example of how to properly use the filename
keyword here, and that should be added.
Anyway, "needs work" to clarify/fix these filename
use issues.
comment:22 follow-up: 24 Changed 8 years ago by
Replying to kcrisman:
sage: var('y') y sage: plot3d(x^2+y^3,(x,-1,1),(y,-1,1),filename='foo') sage: plot3d(x^2+y^3,(x,-1,1),(y,-1,1),filename='foo',viewer='tachyon')
The correct syntax is
sage: plot3d(x^2+y^3,(x,-1,1),(y,-1,1)).show(filename='foo')
comment:23 follow-up: 25 Changed 8 years ago by
Replying to kcrisman:
Here is something minor but still needing attention. The documentation says
- ``filename`` -- string (default: a temp file); file to save the image tobut of course this is now only going to be true in the command line. Which probably makes sense.
No, it doesn't make sense. An explicit filename
should override the default, even in the notebook.
comment:24 follow-up: 28 Changed 8 years ago by
sage: var('y') y sage: plot3d(x^2+y^3,(x,-1,1),(y,-1,1),filename='foo') sage: plot3d(x^2+y^3,(x,-1,1),(y,-1,1),filename='foo',viewer='tachyon')The correct syntax is
sage: plot3d(x^2+y^3,(x,-1,1),(y,-1,1)).show(filename='foo')
Good to know. But that sort of proves my point, then, that the documentation (old and new) doesn't tell me that - in this case, because so many keywords can be passed to the plot and then show up in show()
. Such as what worked in Sage 6.2 and possibly broken by the new display hook
sage: var('y') y sage: plot3d(x^2+y^3,(x,-1,1),(y,-1,1),viewer='jmol') # jmol applet pops up sage: plot3d(x^2+y^3,(x,-1,1),(y,-1,1),viewer='tachyon') # tachyon-generated png pops up in a viewer
If you confirm that this does work properly to generate the filenames in the command line then I'm okay with making sure there are explicit examples that say it doesn't work from the plot command - preferably in the plot command docs!
comment:25 Changed 8 years ago by
Here is something minor but still needing attention. The documentation says
- ``filename`` -- string (default: a temp file); file to save the image tobut of course this is now only going to be true in the command line. Which probably makes sense.
No, it doesn't make sense. An explicit
filename
should override the default, even in the notebook.
Hmm, I guess. But I don't know that there is any easy way to find files in a cell directory in the notebook - though I guess a good use case is to save it in the DATA
directory, where it of course shouldn't show up in the cell output.
comment:26 follow-up: 27 Changed 8 years ago by
Any files created in the current directory are picked up by the notebook, so it would actually work!
comment:27 Changed 8 years ago by
Any files created in the current directory are picked up by the notebook, so it would actually work!
Sorry, I wasn't clear. I meant that the current code in this ticket doesn't allow the filenames, and if one enabled it, it would be kind of useless since there is no access to the cell directory. My counterexample to that is that one might want to use filename=DATA+'foo'
or something to put a graphic file in the data directory, though I haven't tested whether this actually works.
comment:29 Changed 8 years ago by
Replying to kcrisman:
Finally, I don't understand this comment - I'm sorry if I am just thinking inside the box:
# when testing this, modify graphics_filename so that it # returns files which don't automatically share a base name, # e.g.: "import random; i = random.randint(0, 10000)"Was this just a "note to self" or do you mean that, as a tester, I should actually change
graphics_filename
instead of letting it just keep adding one to base names, or ... ?
Doctests mostly check whether a given file can be created, but to check whether that file displays as intended, you'd use an interactive setup not a doctest. So I envision testers using the notebook to run tests, with no way to programmatically tell whether he is doing tests or something else.
And the notebook numbers files by incrementing some counter until they are unique. I.e. starting by sage0.png
then sage1.png
. But the critical point here is that the collision detection takes file extensions into account. So if I were to request e.g. a .foo
file and a .bar
file, I'd get sage0.foo
and sage0.bar
not sage1.bar
. Usually. Unless sage0.bar
already exists at the time the graphics file name is requested, in which case I'd get sage0.foo
but sage1.bar
.
The problematic case I had in mind was one where some viewer uses one file name to derive some other file name. So if a viewer were to open sage0.foo
and from that name infer that it also has to open sage0.bar
, then this would work during test cases, but might fail unexpectedly if a user creates sage0.bar
in some other code inside the same cell.
I'd like to detect such situations. I'd like to make sure that if there is more than one file required for some viewer, then all file names are stated explicitly somewhere, and not inferred by some extension replacement. And that's the point of the comment. I'd like to break the sequential numbering and replace it by random numbers to ensure that inferred file names would break.
So yes, anyone who tests a patch that might be affected by the scenario outlined above should, in my opinion, temporarily modify the code of graphics_filename
to ensure that no file name inference is used. If you can think of a better wording to convey this idea, please let me know. I guess we could also add a link to this comment here, since the whole explanation is too long to put into such a comment, imho.
comment:30 Changed 8 years ago by
In vanilla (i.e. without this patch) sage-6.4.rc0 in the notebook, when doing
cube(viewer="jmol")
I get
/usr/local/src/sage-config/local/lib/python2.7/site-packages/IPython/cor\ e/formatters.py:239: FormatterWarning: Exception in text/plain formatter: Jmol failed to create file '/tmp/tmpE8eQHg/.jmol_images/sage0-size500.jmol.png', see '/home/jdemeyer/.sage/temp/tamiyo/24593/tmp_rBkFiX.txt' for details FormatterWarning, None
(not the clearest error message) followed by a blank red square.
Looking at the file mentioned for details gives
Exception in thread "main" java.lang.UnsupportedClassVersionError: org/jmol/translation/Jmol/en_GB/Messages_en_GB : Unsupported major.minor version 51.0 at java.lang.ClassLoader.defineClass1(Native Method) at java.lang.ClassLoader.defineClass(ClassLoader.java:634) at java.security.SecureClassLoader.defineClass(SecureClassLoader.java:142) at java.net.URLClassLoader.defineClass(URLClassLoader.java:277) at java.net.URLClassLoader.access$000(URLClassLoader.java:73) at java.net.URLClassLoader$1.run(URLClassLoader.java:212) at java.security.AccessController.doPrivileged(Native Method) at java.net.URLClassLoader.findClass(URLClassLoader.java:205) at java.lang.ClassLoader.loadClass(ClassLoader.java:321) at sun.misc.Launcher$AppClassLoader.loadClass(Launcher.java:294) at java.lang.ClassLoader.loadClass(ClassLoader.java:266) at java.lang.Class.forName0(Native Method) at java.lang.Class.forName(Class.java:188) at org.jmol.api.Interface.getInterface(Unknown Source) at org.jmol.i18n.Resource.getResource(Unknown Source) at org.jmol.i18n.GT.addBundle(Unknown Source) at org.jmol.i18n.GT.addBundles(Unknown Source) at org.jmol.i18n.GT.<init>(Unknown Source) at org.jmol.i18n.GT.getTextWrapper(Unknown Source) at org.jmol.i18n.GT._(Unknown Source) at org.openscience.jmol.app.JmolApp.getOptions(Unknown Source) at org.openscience.jmol.app.JmolApp.parseCommandLine(Unknown Source) at org.openscience.jmol.app.JmolData.main(Unknown Source)
Does this mean my Java version is too old?
comment:31 Changed 8 years ago by
Commit: | bd1479d38fe1adb42755ab92499ff8c1ba35592d → 3852a0a23966400e02a14512e97e8ae46e45ec2a |
---|
Branch pushed to git repo; I updated commit sha1. New commits:
3852a0a | Further improve 3d graphics file handling
|
comment:32 Changed 8 years ago by
I think this patch should fix the issues mentioned above (apart from Martin's comment which I decided to simply remove, note that doctests also generate random filenames).
comment:33 follow-up: 36 Changed 8 years ago by
The error in 30 occurs with or without this patch, but I would still like to know what is wrong.
comment:35 Changed 8 years ago by
Reviewers: | Jeroen Demeyer → Jeroen Demeyer, Karl-Dieter Crisman |
---|---|
Status: | needs_work → needs_review |
comment:36 follow-up: 37 Changed 8 years ago by
The error in 30 occurs with or without this patch, but I would still like to know what is wrong.
That indicates to me that your notebook instantiation is somehow corrupted and/or the display hook stuff didn't even come through. Earlier today I had it all working, for example, and now it doesn't even have the extra click boxes in the template, mysteriously. I'm baffled. But that shouldn't happen. Though you may be right about the old java.
comment:37 follow-up: 38 Changed 8 years ago by
cube(viewer="jmol")
The error in 30 occurs with or without this patch, but I would still like to know what is wrong.
That indicates to me that your notebook instantiation is somehow corrupted and/or the display hook stuff didn't even come through. Earlier today I had it all working, for example, and now it doesn't even have the extra click boxes in the template, mysteriously. I'm baffled. But that shouldn't happen. Though you may be right about the old java.
Yeah, I can't replicate this here, though the old java could possible contribute. What happens if you click on the "live 3d" box and then evaluate? That would bypass java creating the static image.
comment:38 Changed 8 years ago by
Replying to kcrisman:
What happens if you click on the "live 3d" box and then evaluate?
When enabling "Load 3-D Live" I get the horrifying error message (it's Halloween, right?)
/usr/local/src/sage-git/local/lib/python2.7/site-packages/IPython/core/f\ ormatters.py:239: FormatterWarning: Exception in text/plain formatter: Jmol failed to create file '/tmp/tmpJEIc27/.jmol_images/sage0-size500.jmol.png', see '/home/jdemeyer/.sage/temp/tamiyo/2874/tmp_IABUbj.txt' for details FormatterWarning, None
then some 3d figure which does work nicely, then the sentence "Right-click to get options menu."
comment:39 Changed 8 years ago by
When enabling "Use java for 3-D" and not "Load 3-D Live" I get a very similar result (with one interesting difference: the right-click menu uses a different font and is in English, with "Load 3-D Live" the menu was in Dutch).
(edit: cannot reproduce this anymore, getting a light grey square like in 40) (edit2: restarting the browser fixed this again)
comment:40 Changed 8 years ago by
When enabling both "Use java for 3-D" and "Load 3-D Live" I get a result similar to 30 except that the blank square is light grey instead of red.
comment:41 Changed 8 years ago by
For the record, I am using
java version "1.6.0_31" OpenJDK Runtime Environment (IcedTea6 1.13.3) (Gentoo build 1.6.0_31-b31) OpenJDK 64-Bit Server VM (build 23.25-b01, mixed mode)
and
Firefox ESR 24.8.0
comment:42 Changed 8 years ago by
I won't have time to look at this over the weekend, nor do I have the jmol/jsmol expertise to diagnose it, but I would recommend putting something about it on the 6.4.rc0 release thread so that Jonathan Gutow sees it.
comment:43 follow-up: 44 Changed 8 years ago by
I don't know if anyone has tried it but I removed the patches associated with #17170 from 6.4.r0 and I still have the issue with viewer='tachyon'
opening a jmol window instead of creating a png. So perhaps someother displayhook-related ticket?
comment:44 follow-up: 47 Changed 8 years ago by
I don't know if anyone has tried it but I removed the patches associated with #17170 from 6.4.r0 and I still have the issue with
viewer='tachyon'
opening a jmol window instead of creating a png. So perhaps someother displayhook-related ticket?
Hmm. Do you know where you first saw this? (I'm not suggesting you do a full bisection! Just wondering.)
comment:45 follow-up: 46 Changed 8 years ago by
I've tried to report the things on this ticket not actually related to this ticket to sage-release. However, now I've forgotten where we were in the review!
comment:46 Changed 8 years ago by
Replying to kcrisman:
I've forgotten where we were in the review!
The commit from comment:31 seems to deal with all outstanding issues from comment:19, so if you review that, I believe we should be fine. I guess Jeroen contributed enough that he should be an author as well. Do you want to file a ticket for the dynamic resizing of applets you mentioned in comment:19?
comment:47 Changed 8 years ago by
Replying to kcrisman:
I don't know if anyone has tried it but I removed the patches associated with #17170 from 6.4.r0 and I still have the issue with
viewer='tachyon'
opening a jmol window instead of creating a png. So perhaps someother displayhook-related ticket?Hmm. Do you know where you first saw this? (I'm not suggesting you do a full bisection! Just wondering.)
I believe on sage-release. But this is not related to this ticket. I only saw your comment:21 above. It should be addressed somewhere.
comment:48 Changed 8 years ago by
comment:49 Changed 8 years ago by
Authors: | Martin von Gagern → Martin von Gagern, Jeroen Demeyer |
---|
For the record: I give positive_review to everything on this ticket not authored by me.
comment:50 follow-up: 51 Changed 8 years ago by
Status: | needs_review → needs_work |
---|
And I am happy with everything except I need to look at the very last patch one more time... okay, looks good other than two questions.
Question - is this a Python syntax I'm not aware of?
def show(self, *, filename=None, **kwds):
In particular, the extra *
mystifies me. (Edit - okay, I found it, but it appears to be Python 3 only? I believe it works in recent Python 2.x since you used it, but I couldn't find a definitive reference - even the Python reference was ambiguous.)
Oh, and maybe add a doctest for this commit with the error raised? Maybe that's too trivial to do... but for some reason the doctest
sage: p.show(viewer='canvas3d')
doesn't raise an error, which makes sense because of the code. But it also is now doctested with the most recent change you made, despite what you just said earlier that making canvas3d guys was too long!
Somehow this should be resolved. I think the best way is to doctest the creation of the file, perhaps by setting a one-time flag or something, rather than the error - as long as we tell people that in the command line it will give an error. (Well, we could doctest both as well!)
comment:51 follow-up: 52 Changed 8 years ago by
Replying to kcrisman:
In particular, the extra
*
mystifies me. (Edit - okay, I found it, but it appears to be Python 3 only? I believe it works in recent Python 2.x since you used it, but I couldn't find a definitive reference - even the Python reference was ambiguous.)
This syntax for keyword-only parameters comes from Python 3 but is supported by Cython even for older versions of Python. In #16607 I suggested support for keyword-only parameters in pure Python code, and there jdemeyer pointed out that Cython support.
comment:52 follow-up: 53 Changed 8 years ago by
Status: | needs_work → needs_info |
---|
In particular, the extra
*
mystifies me. (Edit - okay, I found it, but it appears to be Python 3 only? I believe it works in recent Python 2.x since you used it, but I couldn't find a definitive reference - even the Python reference was ambiguous.)This syntax for keyword-only parameters comes from Python 3 but is supported by Cython even for older versions of Python. In #16607 I suggested support for keyword-only parameters in pure Python code, and there jdemeyer pointed out that Cython support.
Aha.
With this patch, the
filename=
option does work in the notebook.
And it really does save to where needed, awesome! Especially in DATA
could be useful.
despite what you just said earlier that making canvas3d guys was too long!
Okay, sorry, I see what you did now - got a little too excited there. What do you think about testing the warning?
Otherwise this was good teamwork by you two, very nice.
comment:53 Changed 8 years ago by
Replying to kcrisman:
despite what you just said earlier that making canvas3d guys was too long!
Okay, sorry, I see what you did now - got a little too excited there. What do you think about testing the warning?
I'm too confused, what are you (plural) trying to say here?
comment:54 follow-up: 55 Changed 8 years ago by
Here is the short version:
p.show(viewer='canvas3d')
raises an error in the command line but not during doctesting because of the way you wrote the code (creates the file). Perhaps the warning should also be doctested.
comment:55 follow-up: 56 Changed 8 years ago by
Replying to kcrisman:
Perhaps the warning should also be doctested.
That's possible but requires some messing around with DOCTEST_MODE
. Given that it's a minor thing and that Volker has plans to change all this code anyway, I feel that it's not worth the trouble. But if you insist, I will add that test.
PS: I assume you mean "error", not "warning".
comment:56 Changed 8 years ago by
Status: | needs_info → positive_review |
---|
Perhaps the warning should also be doctested.
That's possible but requires some messing around with
DOCTEST_MODE
.
Yes.
Given that it's a minor thing and that Volker has plans to change all this code anyway, I feel that it's not worth the trouble. But if you insist, I will add that test.
Ok.
PS: I assume you mean "error", not "warning".
Yes, though I recall writing both of those a few times and then accidentally closing the browser/getting timed out!
comment:57 Changed 8 years ago by
Status: | positive_review → needs_work |
---|
Conflicts with 6.4, please merge.
comment:58 Changed 8 years ago by
Commit: | 3852a0a23966400e02a14512e97e8ae46e45ec2a → e83f839da144e1d90e495b953ef3f5861d341f41 |
---|
Branch pushed to git repo; I updated commit sha1. New commits:
e83f839 | Merge tag '6.4' into ticket/16640
|
comment:59 Changed 8 years ago by
Status: | needs_work → positive_review |
---|
Done, the conflict was the removal of a redundant import of tmp_filename
.
comment:60 Changed 8 years ago by
Status: | positive_review → needs_work |
---|
sage -t --long src/sage/repl/display/formatter.py ********************************************************************** File "src/sage/repl/display/formatter.py", line 320, in sage.repl.display.formatter.SageNBTextFormatter.__call__ Failed example: fmt(FooGraphics()) Expected: showing graphics '' Got: showing graphics doctest:239: FormatterWarning: Exception in text/plain formatter: [Errno 2] No such file or directory: '/nonexistent.png' **********************************************************************
comment:61 Changed 8 years ago by
Commit: | e83f839da144e1d90e495b953ef3f5861d341f41 → 034fa58b0051be5db6942a76c06010e15a3d664b |
---|
Branch pushed to git repo; I updated commit sha1. New commits:
034fa58 | Fix doctest to use existing file
|
comment:62 Changed 8 years ago by
Status: | needs_work → needs_review |
---|
comment:63 Changed 8 years ago by
Status: | needs_review → positive_review |
---|
comment:64 Changed 8 years ago by
Branch: | u/jdemeyer/ticket/16640 → 034fa58b0051be5db6942a76c06010e15a3d664b |
---|---|
Resolution: | → fixed |
Status: | positive_review → closed |
Note that only the last two commits are new for this ticket; the rest comes from #16645 and its dependencies.
I tested things both in notebook and interactively. java3d failed in both cases, for which I just filed #16647. Apart from that, everything looks fine. In particular it seems as if I got all the file name interdependencies for jmol right.
New commits:
minimal fix to restore functionality
Merge branch 'develop' into ticket/16533-stopgap
graphics_filename: return a tmp_filename() if not in EMBEDDED_MODE
Restore possibly positional arguments.
Merge branch ticket/15515 into ticket/16608 to create ticket/16645.
Always use graphics_filename instead of tmp_filename.
Prevent doctest from leaking file into current working directory.
Include dot in extension for graphics_filename().
Generate names for 3d graphics files independently.