#8632 closed defect (fixed)
.save ignores ymin etc.
Reported by: | damm | Owned by: | was |
---|---|---|---|
Priority: | major | Milestone: | sage-4.6.2 |
Component: | graphics | Keywords: | |
Cc: | kcrisman | Merged in: | sage-4.6.2.alpha2 |
Authors: | Andrey Novoseltsev | Reviewers: | Karl-Dieter Crisman |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description (last modified by )
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
Attachments (2)
Change History (20)
comment:1 Changed 10 years ago by
comment:2 Changed 10 years ago by
- Milestone set to sage-4.6.2
- Status changed from new to needs_review
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.
comment:4 Changed 10 years ago by
- Reviewers set to Karl-Dieter Crisman
- Status changed from needs_review to needs_work
- Work issues set to needs patch
Except we need a patch :-) It could go in the TESTS section; it would need to use the usual temp directory for Sage.
comment:5 follow-up: ↓ 6 Changed 10 years ago by
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.
comment:6 in reply to: ↑ 5 Changed 10 years ago by
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
andsave
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.
Changed 10 years ago by
comment:7 Changed 10 years ago by
- Status changed from needs_work to needs_review
Thank you! How about this patch?
comment:8 Changed 10 years ago by
- Work issues needs patch deleted
I assume the patch depends on #7981, correct?
comment:9 Changed 10 years ago by
Yes!
comment:10 Changed 10 years ago by
- Status changed from needs_review to positive_review
Positive review.
To buildbot: depends on #7981, apply trac_8632_save_ignores_options_from_plot.patch
comment:11 Changed 10 years ago by
Scratch that. Need to change the patch a bit so documentation looks ok - a missing colon.
comment:12 Changed 10 years ago by
Okay, now all is well.
to buildbot: depends on #7981, apply trac_8632_save_ignores_options_from_plot.patch and trac_8632-reviewer.patch
comment:13 Changed 10 years ago by
Thanks for the corrections, sorry for sloppiness!
comment:14 follow-up: ↓ 15 Changed 10 years ago by
Just FYI - still applies fine on 4.6.2.alpha0.
comment:15 in reply to: ↑ 14 Changed 10 years ago by
- Status changed from positive_review to needs_work
- Work issues set to rebase
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
comment:16 Changed 10 years ago by
- Description modified (diff)
- Status changed from needs_work to positive_review
Sorry, I missed the dependency on #7981.
comment:17 Changed 10 years ago by
- Merged in set to sage-4.6.2.alpha2
- Resolution set to fixed
- Status changed from positive_review to closed
comment:18 Changed 10 years ago by
- Work issues rebase deleted
See also #7981.