Ticket #12960 (closed defect: fixed)

Opened 12 months ago

Last modified 12 months ago

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 Download to devel/sage

Attachments

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

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.

Changed 12 months ago by ppurka

apply to devel/sage

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!

Changed 12 months ago by ppurka

apply to devel/sage

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
Note: See TracTickets for help on using tickets.