Opened 7 years ago
Closed 5 years ago
#16325 closed defect (fixed)
implicit_plot does not handle color parameter properly
Reported by: | ppurka | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-7.4 |
Component: | graphics | Keywords: | |
Cc: | kcrisman, paulmasson | Merged in: | |
Authors: | Punarbasu Purkayastha | Reviewers: | Karl-Dieter Crisman, Travis Scrimshaw, Paul Masson |
Report Upstream: | N/A | Work issues: | |
Branch: | daf974e (Commits, GitHub, GitLab) | Commit: | daf974e16c14dd934407f9701cdbcd9ee1544b77 |
Dependencies: | Stopgaps: |
Description (last modified by )
There is no way to handle the color parameter when the fill
option is provided. implicit_plot
should also accept a fillcolor
parameter, just like plot
does, and pass fillcolor
and color
on to region_plot
as incol
and bordercol
, respectively.
Change History (27)
comment:1 Changed 7 years ago by
- Description modified (diff)
comment:2 Changed 7 years ago by
- Branch set to u/ppurka/ticket/16325
- Created changed from 05/11/14 08:48:12 to 05/11/14 08:48:12
- Modified changed from 05/11/14 09:20:14 to 05/11/14 09:20:14
comment:3 Changed 7 years ago by
- Status changed from new to needs_review
comment:4 Changed 7 years ago by
- Cc kcrisman added
- Commit set to 2ecec6b91cce92fc326ed83e1bcb051eb2e4d203
comment:5 Changed 7 years ago by
- Milestone changed from sage-6.3 to sage-6.4
comment:6 Changed 7 years ago by
I'm getting an error, probably because this wasn't added to the list of options.
Failed example: implicit_plot(f, (-3, 3), (-3, 3), fill=True, fillcolor='red') Expected nothing Got: verbose 0 (154: primitive.py, options) WARNING: Ignoring option 'fillcolor'=red verbose 0 (154: primitive.py, options) The allowed options for ContourPlot defined by a 150 x 150 data grid are: cmap the name of a predefined colormap, a list of colors, or an instance of a matplotlib Colormap. Type: import matplotlib.cm; matplotlib.cm.datad.keys() for available colormap names. <snip> **********************************************************************
Yeah, this is just not working either when I try to change that piece. Is there some prereq I'm missing?
comment:7 Changed 7 years ago by
- Reviewers set to Karl-Dieter Crisman
- Status changed from needs_review to needs_work
comment:8 follow-up: ↓ 10 Changed 7 years ago by
I don't know why you are getting such errors. Things are working fine for me.
~/Installations/sage-git» sage -tp --long src/sage/plot too few successful tests, not using stored timings Running doctests with ID 2014-08-16-18-13-09-745f1adf. Sorting sources by runtime so that slower doctests are run first.... Doctesting 51 files using 2 threads. sage -t --long src/sage/plot/graphics.py [411 tests, 195.73 s] sage -t --long src/sage/plot/plot.py [389 tests, 230.06 s] sage -t --long src/sage/plot/line.py [79 tests, 38.26 s] sage -t --long src/sage/plot/contour_plot.py [125 tests, 91.69 s] sage -t --long src/sage/plot/plot3d/implicit_plot3d.py [57 tests, 37.78 s] sage -t --long src/sage/plot/plot3d/base.pyx [269 tests, 30.36 s] sage -t --long src/sage/plot/plot3d/plot3d.py [227 tests, 28.19 s] sage -t --long src/sage/plot/matrix_plot.py [62 tests, 27.53 s] sage -t --long src/sage/plot/point.py [81 tests, 23.88 s] sage -t --long src/sage/plot/complex_plot.pyx [31 tests, 23.32 s] sage -t --long src/sage/plot/polygon.py [71 tests, 18.16 s] sage -t --long src/sage/plot/animate.py [99 tests, 20.08 s] sage -t --long src/sage/plot/circle.py [54 tests, 17.22 s] sage -t --long src/sage/plot/arrow.py [66 tests, 17.25 s] sage -t --long src/sage/plot/plot3d/plot_field3d.py [8 tests, 13.80 s] sage -t --long src/sage/plot/plot3d/parametric_plot3d.py [196 tests, 15.08 s] sage -t --long src/sage/plot/plot_field.py [53 tests, 15.75 s] sage -t --long src/sage/plot/plot3d/tachyon.py [312 tests, 14.86 s] sage -t --long src/sage/plot/density_plot.py [35 tests, 14.73 s] sage -t --long src/sage/plot/plot3d/shapes2.py [109 tests, 11.13 s] sage -t --long src/sage/plot/text.py [42 tests, 9.88 s] sage -t --long src/sage/plot/bezier_path.py [37 tests, 8.35 s] sage -t --long src/sage/plot/misc.py [42 tests, 9.13 s] sage -t --long src/sage/plot/ellipse.py [35 tests, 8.84 s] sage -t --long src/sage/plot/arc.py [39 tests, 8.62 s] sage -t --long src/sage/plot/disk.py [55 tests, 8.50 s] sage -t --long src/sage/plot/plot3d/parametric_surface.pyx [85 tests, 8.03 s] sage -t --long src/sage/plot/colors.py [248 tests, 5.38 s] sage -t --long src/sage/plot/bar_chart.py [22 tests, 6.34 s] sage -t --long src/sage/plot/plot3d/revolution_plot3d.py [20 tests, 6.09 s] sage -t --long src/sage/plot/plot3d/shapes.pyx [159 tests, 6.37 s] sage -t --long src/sage/plot/plot3d/transform.pyx [25 tests, 4.83 s] sage -t --long src/sage/plot/scatter_plot.py [19 tests, 5.11 s] sage -t --long src/sage/plot/plot3d/list_plot3d.py [37 tests, 3.73 s] sage -t --long src/sage/plot/hyperbolic_arc.py [8 tests, 4.27 s] sage -t --long src/sage/plot/plot3d/implicit_surface.pyx [93 tests, 2.65 s] sage -t --long src/sage/plot/step.py [4 tests, 3.44 s] sage -t --long src/sage/plot/plot3d/platonic.py [41 tests, 2.79 s] sage -t --long src/sage/plot/primitive.py [43 tests, 2.90 s] sage -t --long src/sage/plot/plot3d/tri_plot.py [70 tests, 0.98 s] sage -t --long src/sage/plot/hyperbolic_triangle.py [8 tests, 2.53 s] sage -t --long src/sage/plot/plot3d/index_face_set.pyx [48 tests, 0.94 s] sage -t --long src/sage/plot/plot3d/texture.py [61 tests, 0.54 s] sage -t --long src/sage/plot/java3d.py [1 test, 0.05 s] sage -t --long src/sage/plot/plot3d/examples.py [4 tests, 0.59 s] sage -t --long src/sage/plot/plot3d/point_c.pxi [0 tests, 0.00 s] sage -t --long src/sage/plot/plot3d/help.pyx [0 tests, 0.00 s] sage -t --long src/sage/plot/plot3d/all.py [0 tests, 0.00 s] sage -t --long src/sage/plot/plot3d/__init__.py [0 tests, 0.00 s] sage -t --long src/sage/plot/all.py [0 tests, 0.00 s] sage -t --long src/sage/plot/__init__.py [0 tests, 0.00 s] ---------------------------------------------------------------------- All tests passed! ---------------------------------------------------------------------- Total time for all tests: 509.0 seconds cpu time: 813.9 seconds cumulative wall time: 1005.8 seconds ~/Installations/sage-git» sage --version Sage Version 6.4.beta0, Release Date: 2014-08-14 ~/Installations/sage-git» git s On branch ticket/16325 nothing to commit, working directory clean ~/Installations/sage-git» git l -n 6 * eb27cd8 - (HEAD, refs/heads/ticket/16325) simplify bordercolor 11 minutes ago <Punarbasu Purkayastha> * a7382e5 - Merge branch 'u/ppurka/ticket/16325' of trac.sagemath.org:sage into ticket/16325 51 minutes ago <Punarbasu Purkayastha> |\ | * 2ecec6b - (refs/remotes/trac/u/ppurka/ticket/16325) bordercolor must not be a list. Fix this error. 3 months ago <Punarbasu Purkayastha> | * 2b1a65b - Enable fillcolor and bordercolor for implicit_plot 3 months ago <Punarbasu Purkayastha> * | 07b385d - (refs/remotes/origin/develop, refs/heads/develop) Updated Sage version to 6.4.beta0 2 days ago <Volker Braun> * | 974c1c1 - Trac #16807: Overflow error in conversion Integer -> FiniteFieldElement_pari_ffelt 3 days ago <Release Manager> |\ \ %
comment:9 Changed 7 years ago by
- Commit changed from 2ecec6b91cce92fc326ed83e1bcb051eb2e4d203 to eb27cd89962322d64bd187760a919bf35aee33f4
comment:10 in reply to: ↑ 8 Changed 7 years ago by
I don't know why you are getting such errors. Things are working fine for me.
I'll have to try again, then - probably I made a mistake in setting up the branch or something. Won't be immediately. In any case, otherwise I liked the idea! Hopefully it was just something I did.
comment:11 Changed 7 years ago by
Not sure what happened before...
$ ./sage -t src/sage/plot/contour_plot.py too many failed tests, not using stored timings Running doctests with ID 2014-08-22-16-06-05-3f1b7b16. Doctesting 1 file. sage -t src/sage/plot/contour_plot.py [123 tests, 25.81 s] ---------------------------------------------------------------------- All tests passed! ---------------------------------------------------------------------- Total time for all tests: 25.9 seconds cpu time: 25.5 seconds cumulative wall time: 25.8 seconds
Two requests:
- Add an example with a different color and fill color (for the user), if possible one where the function is not a symbolic expression (just to test that branch).
- Technically, I guess both occurrences of
string (default: ``blue``)
should bestring (default: ``'blue'``)
. Is that right? I know you will actually care about getting this right :-)
Otherwise this is great, nice work. Interesting that it didn't work at all before.
comment:12 Changed 5 years ago by
- Branch changed from u/ppurka/ticket/16325 to public/graphics/implicit_plot_color_parameter-16325
- Commit changed from eb27cd89962322d64bd187760a919bf35aee33f4 to 36d71e9af240eefb6bac34cc0ac044496eea5905
- Milestone changed from sage-6.4 to sage-7.3
- Reviewers changed from Karl-Dieter Crisman to Karl-Dieter Crisman, Travis Scrimshaw
- Status changed from needs_work to needs_review
I've made the changes you requested, as well as some other little tidying up of the doc in the file and fixing some trivially failing doctests.
New commits:
666a04b | Merge branch 'u/ppurka/ticket/16325' of trac.sagemath.org:sage into public/graphics/implicit_plot_color_parameter-16325
|
36d71e9 | Trivial doctest changes and some little doc fixes.
|
comment:13 Changed 5 years ago by
Thanks!
fille colors
otherwise, as long as this all still builds then should be positive review - I review your changes modulo that typo.
comment:14 Changed 5 years ago by
- Commit changed from 36d71e9af240eefb6bac34cc0ac044496eea5905 to 1da468df2cb825bc7c26637dc48a534aaffd7809
Branch pushed to git repo; I updated commit sha1. New commits:
1da468d | Fixing typo.
|
comment:15 Changed 5 years ago by
No problem. Done.
comment:16 Changed 5 years ago by
Wow! Thanks for taking care of this guys! I must have forgotten about this ticket.
Lately, I am unable to do any sage development because my laptop overheats and shuts down any time I do anything compute intensive.
comment:17 Changed 5 years ago by
- Cc paulmasson added
- Status changed from needs_review to needs_work
This file has half a dozen merge conflicts with 7.3.beta8, some of them in the code. I'm happy to review when fixed.
comment:18 Changed 5 years ago by
- Commit 1da468df2cb825bc7c26637dc48a534aaffd7809 deleted
Travis, I was going to resolve the merge conflicts here but the branch went missing sometime yesterday evening. Could you push it back up again? Thanks.
comment:19 Changed 5 years ago by
- Commit set to 9d7ba99a0b98a7478808a8c902d447b4ebc60612
Branch pushed to git repo; I updated commit sha1. New commits:
2b1a65b | Enable fillcolor and bordercolor for implicit_plot
|
2ecec6b | bordercolor must not be a list. Fix this error.
|
a7382e5 | Merge branch 'u/ppurka/ticket/16325' of trac.sagemath.org:sage into ticket/16325
|
eb27cd8 | simplify bordercolor
|
666a04b | Merge branch 'u/ppurka/ticket/16325' of trac.sagemath.org:sage into public/graphics/implicit_plot_color_parameter-16325
|
36d71e9 | Trivial doctest changes and some little doc fixes.
|
1da468d | Fixing typo.
|
9d7ba99 | Merge branch 'develop' into public/graphics/implicit_plot_color_parameter-16325
|
comment:20 Changed 5 years ago by
- Status changed from needs_work to needs_review
I managed to find a copy, and I also handled the merge conflict.
comment:21 Changed 5 years ago by
- Commit changed from 9d7ba99a0b98a7478808a8c902d447b4ebc60612 to 7e74f9e06040722a8b2a5532b548460c3029c804
comment:22 Changed 5 years ago by
Two doctests were failing which I fixed. They all pass now. New functionality for plots works as expected.
I added sphinx plots for new examples. Documentation builds.
Who acts as reviewer now?
comment:23 Changed 5 years ago by
- Milestone changed from sage-7.3 to sage-7.4
- Reviewers changed from Karl-Dieter Crisman, Travis Scrimshaw to Karl-Dieter Crisman, Travis Scrimshaw, Paul Masson
I can, and the only thing that I think needs to be done is removing the # long time
from the .. PLOT::
code (at least, I don't see the purpose of it being there).
comment:24 Changed 5 years ago by
- Commit changed from 7e74f9e06040722a8b2a5532b548460c3029c804 to daf974e16c14dd934407f9701cdbcd9ee1544b77
Branch pushed to git repo; I updated commit sha1. New commits:
daf974e | More cleanup
|
comment:25 Changed 5 years ago by
Removed # long time
from four ..PLOT::
s in the file.
comment:27 Changed 5 years ago by
- Branch changed from public/graphics/implicit_plot_color_parameter-16325 to daf974e16c14dd934407f9701cdbcd9ee1544b77
- Resolution set to fixed
- Status changed from positive_review to closed
Sorry I'm adding myself to all these but not finding time to review them... I need to set aside a couple entire days after I finish grading finals just to review your plotting improvements, there are so many! (Which is good.)
New commits:
Enable fillcolor and bordercolor for implicit_plot
bordercolor must not be a list. Fix this error.