.save ignores ymin etc.

[damm]

a sage (4.3.3) notebook shows the correct picture of

plot(x^2-5,(x,0,5),ymin=0)

The save method ignores the ymin parameter:

plot(x^2-5,(x,0,5),ymin=0).save("/tmp/test.png")

Dependency: #7981

[novoselt]

See also #7981.

[novoselt]

With the current patch at #7981 this problem is gone. The graph goes a bit below ymin=0, but it happens in both cases in the same way, so save does not ignore the parameter anymore.

[novoselt]

Easy review with #7981 applied ;-)

[kcrisman]

Except we need a patch :-) It could go in the TESTS section; it would need to use the usual temp directory for Sage.

[novoselt]

Could you please remind me what is the usual temp directory for Sage?

Also, what exactly should the test do? How do I verify that images from show and save are the same? It seems like a waste of time on tests if it is only checked that these commands don't raise an exception - they were working before as well, just not as they should.

[kcrisman]

Replying to novoselt:

Could you please remind me what is the usual temp directory for Sage?

See line 1732 of your patch for #7981 ;-) - kwds.setdefault('filename', sage.misc.misc.tmp_filename() + '.png')

Also, what exactly should the test do? How do I verify that images from show and save are the same? It seems like a waste of time on tests if it is only checked that these commands don't raise an exception - they were working before as well, just not as they should.

Sadly, we can't do that yet. (Matplotlib apparently does do this with Nose, but we don't have the capability yet.) Partly this could be useful for the future day when we CAN check things like this...

But for now the point is that at least someone can visually verify that there is a different min than $y=-5$ if they care to look. We want to document that we have done something about the particular one.

Alternately, you could try to ask a release manager to close this as a duplicate of #7981. Personally, I think it would be worth adding an example to the save documentation that one can choose to either put the commands in .save(foo=bar) or to pass it one from plot(f,foo=bar); that could be useful for a complete newbie to the code to see, so that they don't have to follow code around.

[novoselt]

Thank you! How about this patch?

[kcrisman]

I assume the patch depends on #7981, correct?

[novoselt]

Yes!

[kcrisman]

Positive review.

To buildbot: depends on #7981, apply trac_8632_save_ignores_options_from_plot.patch

[kcrisman]

Scratch that. Need to change the patch a bit so documentation looks ok - a missing colon.

[kcrisman]

reviewer patch

[kcrisman]

Okay, now all is well.

to buildbot: depends on #7981, apply trac_8632_save_ignores_options_from_plot.patch and trac_8632-reviewer.patch

[novoselt]

Thanks for the corrections, sorry for sloppiness!

[kcrisman]

Just FYI - still applies fine on 4.6.2.alpha0.

[jdemeyer]

Replying to kcrisman:

Just FYI - still applies fine on 4.6.2.alpha0.

Actually, it doesn't:

patching file sage/plot/plot.py
Hunk #1 FAILED at 2394.
1 out of 1 hunk FAILED -- saving rejects to file sage/plot/plot.py.rej

[jdemeyer]

Sorry, I missed the dependency on #7981.