Opened 7 years ago
Closed 7 years ago
#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: | Merged in: | sage-5.1.beta2 | |
Authors: | Punarbasu Purkayastha | Reviewers: | Karl-Dieter Crisman |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description (last modified by )
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 (2)
Change History (12)
comment:1 Changed 7 years ago by
- Description modified (diff)
- Status changed from new to needs_review
comment:2 Changed 7 years ago by
comment:3 Changed 7 years ago by
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 7 years ago by
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 7 years ago by
- Status changed from needs_review to needs_work
- Work issues set to check show_legend
comment:6 Changed 7 years ago by
- Description modified (diff)
- Status changed from needs_work to needs_review
- 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 7 years ago by
- Keywords sd40.5 added
- Reviewers set to Karl-Dieter Crisman
- Status changed from needs_review to needs_work
Yes, this makes a lot more sense. Needs doctests!
comment:8 Changed 7 years ago by
- Status changed from needs_work to needs_review
Updated the patch with a test.
comment:9 Changed 7 years ago by
- Status changed from needs_review to positive_review
Looks great, thanks for the good work!
comment:10 Changed 7 years ago by
- Merged in set to sage-5.1.beta2
- Resolution set to fixed
- Status changed from positive_review to closed
Hmm.. the matplotlib arguments are also changed it seems. I will revert that.