Ticket #12960 (closed defect: fixed)
legend not properly set in Graphics().matplotlib()
| Reported by: | ppurka | Owned by: | jason, was |
|---|---|---|---|
| Priority: | major | Milestone: | sage-5.1 |
| Component: | graphics | Keywords: | legend matplotlib sd40.5 |
| Cc: | Work issues: | ||
| Report Upstream: | N/A | Reviewers: | Karl-Dieter Crisman |
| Authors: | Punarbasu Purkayastha | Merged in: | sage-5.1.beta2 |
| Dependencies: | Stopgaps: |
Description (last modified by ppurka) (diff)
Bug :) Should be easy to fix.
sage: q = plot(x, legend_label='aha')
sage: q.legend(True)
sage: qm = q.matplotlib()
---------------------------------------------------------------------------
KeyError Traceback (most recent call last)
/home/punarbasu/Installations/sage-5.0.rc0.11080/devel/sage-main/<ipython console> in <module>()
/home/punarbasu/Installations/sage-5.0.rc0.11080/local/lib/python2.7/site-packages/sage/plot/graphics.pyc in matplotlib(self, filename, xmin, xmax, ymin, ymax, figsize, figure, sub, axes, axes_labels, fontsize, frame, verify, aspect_ratio, gridlines, gridlinesstyle, vgridlinesstyle, hgridlinesstyle, show_legend, legend_options, axes_pad, ticks_integer, tick_formatter, ticks)
1714 lopts.update(legend_options)
1715 lopts.update(self._legend_opts)
-> 1716 prop = FontProperties(family=lopts.pop('font_family'), weight=lopts.pop('font_weight'), \
1717 size=lopts.pop('font_size'), style=lopts.pop('font_style'), variant=lopts.pop('font_variant'))
1718 color = lopts.pop('back_color')
KeyError: 'font_family'
sage:
Apply trac_12960-fix_legend.2.patch to devel/sage
Attachments
Change History
comment:1 Changed 12 months ago by ppurka
- Status changed from new to needs_review
- Description modified (diff)
comment:2 Changed 12 months ago by ppurka
Hmm.. the matplotlib arguments are also changed it seems. I will revert that.
comment:3 Changed 12 months ago by kcrisman
Does this really work? I'm unclear as to how this would work in the case show is None. Or maybe it's not supposed to then, since it's not being shown... in which case why not just put this in the matplotlib definition, which currently has legend_options={}? Or maybe with a @suboptions decorator like in show? This seems more "Sage-ic".
comment:4 Changed 12 months ago by ppurka
Whoever wrote the legend code probably didn't want unnecessary initializations. I kept it the same, that is the legend_opts don't get populated unless it is absolutely required. Otherwise we could just initialize it in __init__ instead of passing @options or @suboptions all over the place.
If show_legend is None, then of course, nothing happens. If it is set to True, then something weird can happen. I will check this case.
comment:5 Changed 12 months ago by ppurka
- Status changed from needs_review to needs_work
- Work issues set to check show_legend
comment:6 Changed 12 months ago by ppurka
- Status changed from needs_work to needs_review
- Description modified (diff)
- Work issues check show_legend deleted
Ok. This is a much better fix since it takes care of all cases, for example if you pass an incomplete set of options in the legend={} argument, set show_legend=True, etc.
comment:7 Changed 12 months ago by kcrisman
- Keywords sd40.5 added
- Reviewers set to Karl-Dieter Crisman
- Status changed from needs_review to needs_work
- Authors set to Punarbasu Purkayastha
Yes, this makes a lot more sense. Needs doctests!
comment:8 Changed 12 months ago by ppurka
- Status changed from needs_work to needs_review
Updated the patch with a test.
comment:9 Changed 12 months ago by kcrisman
- Status changed from needs_review to positive_review
Looks great, thanks for the good work!
comment:10 Changed 12 months ago by jdemeyer
- Status changed from positive_review to closed
- Resolution set to fixed
- Merged in set to sage-5.1.beta2

