Ticket #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 | Work issues: | |
| Report Upstream: | N/A | Reviewers: | William Stein |
| Authors: | Karl-Dieter Crisman | Merged in: | sage-5.1.beta5 |
| Dependencies: | #12810 | Stopgaps: |
Description (last modified by kcrisman) (diff)
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.
Attachments
-
trac_12605-circle-color-graphplot-doc-rebased.patch
(22.2 KB) -
added by kcrisman 12 months ago.
Change History
comment:2 Changed 14 months ago by kcrisman
- Status changed from new to needs_review
- Authors set to Karl-Dieter Crisman
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 12 months 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 12 months ago by jdemeyer
- Reviewers set to William Stein
- Description modified (diff)
- 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 12 months 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 12 months ago by kcrisman
- Status changed from needs_work to positive_review
Ok, should be good to go.
comment:9 follow-up: ↓ 10 Changed 12 months ago by jdemeyer
Also, you might want to fix the trailing whitespace the Patchbot reports.
comment:10 in reply to: ↑ 9 Changed 12 months 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 :)
Changed 12 months ago by kcrisman
-
attachment
trac_12605-circle-color-graphplot-doc-rebased.patch
added
comment:11 Changed 12 months ago by kcrisman
- Status changed from needs_work to positive_review
- Dependencies set to #12810
- Description modified (diff)
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 11 months ago by jdemeyer
- Status changed from positive_review to closed
- Resolution set to fixed
- Merged in set to sage-5.1.beta5
comment:13 Changed 11 months 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 11 months 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.
