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:

Status badges

Description (last modified by jdemeyer)

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

Apply 11677_polygon_fill_flat.patch.

Attachments (3)

trac_11677_fillFixes.patch (2.0 KB) - added by peter.story 10 years ago.
trac_11677-reviewer.patch (1.2 KB) - added by kcrisman 10 years ago.
apply after fix fill patch
11677_polygon_fill_flat.patch (2.7 KB) - added by jdemeyer 10 years ago.
Rebased to sage-5.0.beta1, apply only this

Download all attachments as: .zip

Change History (17)

comment:1 Changed 10 years ago by kcrisman

  • Authors set to Karl-Dieter Crisman

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:

    EXAMPLES:
        
    Note this should normally be used indirectly via ``circle``::
        
        sage: from sage.plot.polygon import Polygon   

which I thought had another ticket, but maybe it doesn't...

comment:2 Changed 10 years ago by kcrisman

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

  • Description modified (diff)

Oops! The default fill should be True, of course.

Changed 10 years ago by peter.story

comment:4 Changed 10 years ago by peter.story

  • Authors changed from Karl-Dieter Crisman to Karl-Dieter Crisman, Kenny Smith, Peter Story
  • 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 kcrisman

See also #12297.

comment:6 Changed 10 years ago by ksmith

  • Authors changed from Karl-Dieter Crisman, Kenny Smith, Peter Story to Karl-Dieter Crisman, Kenneth Smith, Peter Story

comment:7 Changed 10 years ago by kcrisman

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

Changed 10 years ago by kcrisman

apply after fix fill patch

comment:8 Changed 10 years ago by kcrisman

  • Description modified (diff)

comment:9 Changed 10 years ago by ksmith

  • Keywords sd35.5 added

comment:10 Changed 10 years ago by jdemeyer

  • Milestone changed from sage-4.8 to sage-5.0

comment:11 Changed 10 years ago by jdemeyer

  • 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

Changed 10 years ago by jdemeyer

Rebased to sage-5.0.beta1, apply only this

comment:12 Changed 10 years ago by jdemeyer

  • Description modified (diff)
  • Status changed from needs_work to positive_review

comment:13 Changed 10 years ago by kcrisman

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 jdemeyer

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