Opened 9 years ago

Closed 9 years ago

Last modified 9 years ago

#12605 closed defect (fixed)

Change the default color of circle and ellipses, etc. back to be the same shade of blue as for all other graphics objects

Reported by: was Owned by: jason, was
Priority: minor Milestone: sage-5.1
Component: graphics Keywords:
Cc: kcrisman Merged in: sage-5.1.beta5
Authors: Karl-Dieter Crisman Reviewers: William Stein
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: #12810 Stopgaps:

Status badges

Description (last modified by kcrisman)

We'll have to check how to keep the colors black for the default wherever this is used in graphs, unless it shouldn't be.


Apply trac_12605-circle-color-graphplot-doc-rebased.patch.

Attachments (1)

trac_12605-circle-color-graphplot-doc-rebased.patch (22.2 KB) - added by kcrisman 9 years ago.

Download all attachments as: .zip

Change History (15)

comment:1 Changed 9 years ago by kcrisman

  • Cc kcrisman added

comment:2 Changed 9 years ago by kcrisman

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

Okay, I ended up making some doc prettier and adding graph plotting to the reference manual (although it's added to two .rst files, there is only one additional html file created in the manual). I believe that this correctly addresses the original problem while keeping graphs looking the same, but I'd appreciate any reviewer really comparing the "live" doc for a significant number of plotted graphs in the reference manual.

comment:3 Changed 9 years ago by was

  • Status changed from needs_review to positive_review

Looks good to me. Test pass. Lots of plots I plotted look good, etc. And I like the patch.

comment:4 Changed 9 years ago by jdemeyer

  • Description modified (diff)
  • Reviewers set to William Stein
  • Summary changed from change the default color of cirlce and ellipses, etc. (?) back to be the same shade of blue as for all other graphics objects to Change the default color of circle and ellipses, etc. back to be the same shade of blue as for all other graphics objects

comment:5 Changed 9 years ago by jdemeyer

  • Status changed from positive_review to needs_work

This needs to be rebased to sage-5.1.beta1:

applying trac_12605-circle-color-graphplot-doc.patch
patching file sage/graphs/graph_plot.py
Hunk #2 FAILED at 106
Hunk #4 FAILED at 139
Hunk #5 succeeded at 151 with fuzz 2 (offset 0 lines).
2 out of 20 hunks FAILED -- saving rejects to file sage/graphs/graph_plot.py.rej

comment:6 Changed 9 years ago by kcrisman

  • Status changed from needs_work to positive_review

Ok, should be good to go.

comment:7 Changed 9 years ago by jdemeyer

This patch conflicts with #12810.

comment:8 Changed 9 years ago by jdemeyer

  • Status changed from positive_review to needs_work

comment:9 follow-up: Changed 9 years ago by jdemeyer

Also, you might want to fix the trailing whitespace the Patchbot reports.

comment:10 in reply to: ↑ 9 Changed 9 years ago by kcrisman

Also, you might want to fix the trailing whitespace the Patchbot reports.

Okay, I didn't realize how that worked before. My editor doesn't actually show this or remove it, though it at least never introduces tabs :)

comment:11 Changed 9 years ago by kcrisman

  • Dependencies set to #12810
  • Description modified (diff)
  • Status changed from needs_work to positive_review

Should be good to go, all rebased AND on 5.1.beta1 still. Apply trac_12605-circle-color-graphplot-doc-rebased.patch

comment:12 Changed 9 years ago by jdemeyer

  • Merged in set to sage-5.1.beta5
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:13 Changed 9 years ago by jason

kcrisman: I guess a different ticket would be changing rgbcolor to just color. None of the colors I saw in the patch were actually R,G,B tuples...

comment:14 Changed 9 years ago by kcrisman

Well, you might be right that this is totally inconsistent. I think that that was a design decision a looooong time ago, and I don't quite know where we all use rgbcolor versus color. Reminds me of the difference between alpha (2d) and opacity (3d) which is sort of silly. Anyway, I'm not thinking about this today.

Note: See TracTickets for help on using tickets.