Opened 10 years ago
Closed 10 years ago
#11677 closed defect (fixed)
Polygon fill doesn't work
Reported by: | kcrisman | Owned by: | jason, was |
---|---|---|---|
Priority: | minor | Milestone: | sage-5.0 |
Component: | graphics | Keywords: | beginner sd35.5 |
Cc: | jason | Merged in: | sage-5.0.beta2 |
Authors: | Karl-Dieter Crisman, Kenneth Smith, Peter Story | Reviewers: | Peter Story, Kenneth Smith, Karl-Dieter Crisman |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description (last modified by )
From this sage-support thread:
Hello,
------- ----------- P = polygon([[1,2], [5,6], [5,0]], fill = False, color='red') P ------- -----------
still fills the polygon, is there another way to turn off color fill. Thanks. -Giri
The fix is to add
z = int(options.pop('zorder', 1)) p.set_alpha(a) + f = options.pop('fill', True) + p.set_fill(f) c = to_mpl_color(options['rgbcolor']) p.set_edgecolor(c)
in Polygon._render_on_subplot
in sage/plot/polygon.py.
Notice that this means to actually get such a polygon, you would need to set the thickness. Is that a bug? We do intentionally have the thickness 0 in the @options
.
sage: P = polygon([[1,2], [5,6], [5,0]], fill = False, thickness=1) ; P sage: P = polygon([[1,2], [5,6], [5,0]], fill = False) ; P
Attachments (3)
Change History (17)
comment:1 Changed 10 years ago by
comment:2 Changed 10 years ago by
- Keywords beginner added
Nope, turned out that was #10965. Let's make fill work here too, saving other options for another ticket.
comment:3 Changed 10 years ago by
- Description modified (diff)
Oops! The default fill should be True, of course.
Changed 10 years ago by
comment:4 Changed 10 years ago by
- Status changed from new to needs_review
The 'fill' argument now works. Also, by default, 'thickness' is 0. However, if you specify fill=false, 'thickness' will default to 1.
comment:5 Changed 10 years ago by
See also #12297.
comment:6 Changed 10 years ago by
comment:7 Changed 10 years ago by
- Reviewers set to Peter Story, Kenneth Smith, Karl-Dieter Crisman
- Status changed from needs_review to positive_review
Okay, this looks good, and doesn't seem to have any bugs that weren't already there (see #12297). I've attached a reviewer patch which adds some doctests.
I'm going to consider that Peter and Kenneth verified my original two-liner, and I give positive review to the rest.
comment:8 Changed 10 years ago by
- Description modified (diff)
comment:9 Changed 10 years ago by
- Keywords sd35.5 added
comment:10 Changed 10 years ago by
- Milestone changed from sage-4.8 to sage-5.0
comment:11 Changed 10 years ago by
- Status changed from positive_review to needs_work
This should be rebased to sage-4.8.rc0:
jdemeyer@sage:/scratch/jdemeyer/merger/sage-4.8.rc0/devel/sage$ hg qimport -P http://trac.sagemath.org/sage_trac/raw-attachment/ticket/11677/trac_11677_fillFixes.patch adding trac_11677_fillFixes.patch to series file applying trac_11677_fillFixes.patch patching file sage/plot/polygon.py Hunk #3 FAILED at 271 1 out of 4 hunks FAILED -- saving rejects to file sage/plot/polygon.py.rej patch failed, unable to continue (try -v) patch failed, rejects left in working dir errors during apply, please fix and refresh trac_11677_fillFixes.patch
comment:12 Changed 10 years ago by
- Description modified (diff)
- Status changed from needs_work to positive_review
comment:13 Changed 10 years ago by
Thanks, Jeroen! My students have started classes now, so I didn't think they'd have time to get to it for a while, but this is welcome assistance.
comment:14 Changed 10 years ago by
- Merged in set to sage-5.0.beta2
- Resolution set to fixed
- Status changed from positive_review to closed
Might be worth looking at this file a little more too. There are lots of interesting mpl options like hash one could set, and there is at least one typo:
which I thought had another ticket, but maybe it doesn't...