Opened 9 years ago

Closed 9 years ago

#12620 closed defect (fixed)

problems with bezier3d command

Reported by: was Owned by: jason, was
Priority: minor Milestone: sage-5.0
Component: graphics Keywords:
Cc: kcrisman Merged in: sage-5.0.beta14
Authors: Karl-Dieter Crisman Reviewers: Michael Orlitzky
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description

  • the default color is "red", but it should be the same as for all other plots (i.e., that blueish).
  • The docstring also has this line:
            -  ``color`` - a word that describes a color
    

Yes, you can use a string to describe a color, but there are many other ways. change this to be the same as for other commands.

Attachments (1)

trac_12620-beziercolor.patch (709 bytes) - added by kcrisman 9 years ago.

Download all attachments as: .zip

Change History (8)

comment:1 Changed 9 years ago by kcrisman

  • Cc kcrisman added

comment:2 Changed 9 years ago by kcrisman

There are all kinds of other problems in shapes2.py and shapes.pyx along these lines. For instance, the Box command claims its default is black, but it is indeed blue, while pretty much every command in shapes2 has the same line as this docstring.

I suggest that the following very minimalist patch be done here, and that another ticket be opened for fixing all that doc. What do others think?

Changed 9 years ago by kcrisman

comment:3 Changed 9 years ago by kcrisman

  • Status changed from new to needs_review

Apply trac_12620-beziercolor.patch. Open to suggestions for improvement (are doctests needed for this change, for instance?).

comment:4 Changed 9 years ago by mjo

  • Reviewers set to Michael Orlitzky
  • Status changed from needs_review to positive_review

I looked into adding a doctest that checks the color of the bezier spline against e.g. plot(sin(x)). You have to do enough gymnastics that I don't think it's worth it for this obviously-correct change.

If we wanted to do that, I think it would make sense to create a separate unit test checking that all plotting commands use the same default color, so that we don't have 297592 tests to make sure things are blue.

Anyway, we should open another ticket to fix those docstrings.

comment:5 follow-up: Changed 9 years ago by kcrisman

  • Authors set to Karl-Dieter Crisman

Open it and hopefully it's one of a number of things I can do at the next Bug Days.

comment:6 in reply to: ↑ 5 Changed 9 years ago by mjo

Replying to kcrisman:

Open it and hopefully it's one of a number of things I can do at the next Bug Days.

Ok, made this #12844.

comment:7 Changed 9 years ago by jdemeyer

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