Opened 11 years ago

Closed 7 years ago

#6367 closed defect (fixed)

polygon2d -- several issues: typo in docs, shouldn't have been renamed

Reported by: was Owned by: was
Priority: major Milestone: sage-5.4.1
Component: graphics Keywords:
Cc: ksmith Merged in: sage-5.4.1.rc0
Authors: Karl-Dieter Crisman, Kenneth Smith Reviewers: Volker Braun
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Description

The help for polygon2d? says:

 Type ``polygon.options`` for a dictionary of the default 
    options for polygons.  You can change this to change 

but it should say

 Type ``polygon2d.options`` for a dictionary of the default 
    options for polygons.  You can change this to change 

And for the record I think it is unfortunate that somebody changed the function name from polygon to polygon2d, since that it inconsistent with almost all the rest of plotting in Sage, where the 2d version of the function doesn't specifically say it is 2d (e.g., plot, line, text, etc.) Well, there is line2d, to add to the confusion. The design of Sage graphics is *supposed* to follow the naming scheme in Matheamtica. In Mathematica there is Polygon and Polygon3D. That's what Sage should have.

To resolve this patch, either fix the docstring or change the name back to polygon (not polygon2d). I prefer the latter for consistency with the rest fo the design of sage 2d graphics.

Attachments (1)

trac_6367-polygon.patch (1.4 KB) - added by kcrisman 8 years ago.

Download all attachments as: .zip

Change History (8)

comment:1 Changed 10 years ago by kcrisman

  • Report Upstream set to N/A

For the record, this caused some actual problems for a potential Sage user recently, so it should get fixed - and soon. Probably we'll need to make polygon=polygon2d (for 2d input) for deprecation reasons, for now.

comment:2 follow-up: Changed 8 years ago by kcrisman

  • Authors set to Karl-Dieter Crisman
  • Status changed from new to needs_review

Unfortunately this is pretty hard to change since polygon is expected to work for two and three D input. This patch does take care of the problem.

Changed 8 years ago by kcrisman

comment:3 in reply to: ↑ 2 Changed 8 years ago by kcrisman

Unfortunately this is pretty hard to change since polygon is expected to work for two and three D input.

What I mean, of course, is that it would be hard to change since users probably are using it this way a lot and the deprecation period and all simply isn't worth the effort at this point. Anyway, needs review.

comment:4 Changed 8 years ago by kcrisman

  • Authors changed from Karl-Dieter Crisman to Karl-Dieter Crisman, Kenneth Smith
  • Cc ksmith added

Oops, totally forgot about #12214, which is a dup of this. Given that #12214 is virtually the same ticket, this one is much older, and the patch is quite similar, I'm adding the author there to this ticket and closing that one.

comment:5 Changed 7 years ago by vbraun

  • Reviewers set to Volker Braun
  • Status changed from needs_review to positive_review

Looks good to me!

comment:6 Changed 7 years ago by jdemeyer

  • Milestone changed from sage-5.4 to sage-5.4.1

comment:7 Changed 7 years ago by jdemeyer

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