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 ppurka)

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)

trac_12960-fix_legend.patch (1.0 KB) - added by ppurka 7 years ago.
apply to devel/sage
trac_12960-fix_legend.2.patch (2.5 KB) - added by ppurka 7 years ago.
apply to devel/sage

Download all attachments as: .zip

Change History (12)

comment:1 Changed 7 years ago by ppurka

  • Description modified (diff)
  • Status changed from new to needs_review

comment:2 Changed 7 years ago by ppurka

Hmm.. the matplotlib arguments are also changed it seems. I will revert that.

Changed 7 years ago by ppurka

apply to devel/sage

comment:3 Changed 7 years 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 7 years 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 7 years ago by ppurka

  • Status changed from needs_review to needs_work
  • Work issues set to check show_legend

comment:6 Changed 7 years ago by ppurka

  • 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 kcrisman

  • Authors set to Punarbasu Purkayastha
  • 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!

Changed 7 years ago by ppurka

apply to devel/sage

comment:8 Changed 7 years ago by ppurka

  • Status changed from needs_work to needs_review

Updated the patch with a test.

comment:9 Changed 7 years ago by kcrisman

  • Status changed from needs_review to positive_review

Looks great, thanks for the good work!

comment:10 Changed 7 years ago by jdemeyer

  • Merged in set to sage-5.1.beta2
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.