Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

legend not properly set in Graphics().matplotlib() #12960

Closed
ppurka opened this issue May 17, 2012 · 17 comments
Closed

legend not properly set in Graphics().matplotlib() #12960

ppurka opened this issue May 17, 2012 · 17 comments

Comments

@ppurka
Copy link
Member

ppurka commented May 17, 2012

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 attachment: trac_12960-fix_legend.2.patch to devel/sage

Component: graphics

Keywords: legend matplotlib sd40.5

Author: Punarbasu Purkayastha

Reviewer: Karl-Dieter Crisman

Merged: sage-5.1.beta2

Issue created by migration from https://trac.sagemath.org/ticket/12960

@ppurka

This comment has been minimized.

@ppurka
Copy link
Member Author

ppurka commented May 17, 2012

comment:2

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

@ppurka
Copy link
Member Author

ppurka commented May 17, 2012

Attachment: trac_12960-fix_legend.patch.gz

apply to devel/sage

@kcrisman
Copy link
Member

comment:3

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".

@ppurka
Copy link
Member Author

ppurka commented May 17, 2012

comment:4

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.

@ppurka
Copy link
Member Author

ppurka commented May 17, 2012

Work Issues: check show_legend

@ppurka
Copy link
Member Author

ppurka commented May 18, 2012

Changed work issues from check show_legend to none

@ppurka

This comment has been minimized.

@ppurka
Copy link
Member Author

ppurka commented May 18, 2012

comment:6

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.

@kcrisman
Copy link
Member

comment:7

Yes, this makes a lot more sense. Needs doctests!

@kcrisman
Copy link
Member

Changed keywords from legend matplotlib to legend matplotlib sd40.5

@kcrisman
Copy link
Member

Reviewer: Karl-Dieter Crisman

@kcrisman
Copy link
Member

Author: Punarbasu Purkayastha

@ppurka
Copy link
Member Author

ppurka commented May 25, 2012

Attachment: trac_12960-fix_legend.2.patch.gz

apply to devel/sage

@ppurka
Copy link
Member Author

ppurka commented May 25, 2012

comment:8

Updated the patch with a test.

@kcrisman
Copy link
Member

comment:9

Looks great, thanks for the good work!

@jdemeyer
Copy link

jdemeyer commented Jun 2, 2012

Merged: sage-5.1.beta2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants