Opened 6 years ago
Closed 6 years ago
#14632 closed defect (fixed)
make generic_graph.plot() pass its options to show
Reported by: | ppurka | Owned by: | jason, ncohen, rlm |
---|---|---|---|
Priority: | major | Milestone: | sage-5.10 |
Component: | graph theory | Keywords: | |
Cc: | ncohen | Merged in: | sage-5.10.rc0 |
Authors: | Punarbasu Purkayastha | Reviewers: | Nathann Cohen |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description (last modified by )
Essentially what the title says. If it doesn't support the option, then there are two ways it can be handled:
- it will give an error from show just like the other plot functions do.
- it will check the dict from sage.plot.graphics.Graphics.SHOW_DEFAULT and raise an error from the generic_graph.plot() or generic_graph.graphplot() itself.
Apply to devel/sage: trac_14632-generic_graph_plot.patch
Attachments (1)
Change History (17)
comment:1 Changed 6 years ago by
comment:2 Changed 6 years ago by
- Cc ncohen added
- Description modified (diff)
- Status changed from new to needs_review
Added a patch. Please check that none of your graph plots are broken :)
comment:3 Changed 6 years ago by
- Status changed from needs_review to needs_info
Hmmmm... What can we do about that ?
sage: graphs.PetersenGraph().plot(aewarehaerhaerherh=8)
This is the line that used to return a warning
sage: graphs.PetersenGraph().plot(aewaerh=8) /home/ncohen/.Sage/local/lib/python2.7/site-packages/sage/graphs/generic_graph.py:14263: DeprecationWarning: You provided aewaerh as an argument to a function which has always silently ignored its inputs. This method may soon be updated so that the method raises an exception instead of this warning, which will break your code : to be on the safe side, update it ! See http://trac.sagemath.org/13891 for details. return GraphPlot(graph=self, options=options)
Could we check that all arguments given as input are actually used somewhere ?
Nathann
comment:4 Changed 6 years ago by
Added a check for incorrect arguments. I check against graphplot_options
and the show options that are already extracted.
comment:5 Changed 6 years ago by
- Status changed from needs_info to needs_review
comment:6 Changed 6 years ago by
- Reviewers set to Nathann Cohen
- Status changed from needs_review to positive_review
Coooooooooooooooool ! Thank you very much :-)
Nathann
comment:7 Changed 6 years ago by
That was fast :-O
Thanks!
comment:8 follow-up: ↓ 9 Changed 6 years ago by
Come on thepatch is 5kb long. Let's not bury those for months :-P
Nathann
comment:9 in reply to: ↑ 8 ; follow-up: ↓ 10 Changed 6 years ago by
Replying to ncohen:
Come on thepatch is 5kb long. Let's not bury those for months
:-P
Nathann
Thanks. You should learn some of the graphics code too! Then you can review my patches ;-)
. Anyway, I updated the patch with a one character change that I didn't notice for days.
comment:10 in reply to: ↑ 9 ; follow-up: ↓ 11 Changed 6 years ago by
Thanks. You should learn some of the graphics code too! Then you can review my patches
;-)
.
I'am scaaared of graphics T_T
Anyway, I updated the patch with a one character change that I didn't notice for days.
Can we submit GIT patches already ? O_o
Nathann
comment:11 in reply to: ↑ 10 Changed 6 years ago by
Replying to ncohen:
Can we submit GIT patches already ?
O_o
Nathann
unfortunately, no. That's just my repo to keep track of my patches. I need to apply a bunch of them every time I need to do any work.
comment:12 follow-up: ↓ 13 Changed 6 years ago by
Oh. Do you have many patches waiting for a review ?
Nathann
comment:13 in reply to: ↑ 12 ; follow-up: ↓ 14 Changed 6 years ago by
Replying to ncohen:
Oh. Do you have many patches waiting for a review ?
Nathann
Yeah. Many of them. :-(
comment:14 in reply to: ↑ 13 Changed 6 years ago by
Yeah. Many of them.
:-(
I'll regret that...
--> give me the numbers please :-P
Nathann_who_secretly_(actually_not)_hates_graphics
comment:15 Changed 6 years ago by
Well, some of them are trivial but some are not so trivial :)
You can get the numbers from the file names here.
As for the patches themselves, I would say that
- You can ignore the first six patches because the first two were being reviewed, while the next four will probably never get in.
- Trivial patch: #13690
- For improving consistency the tickets are #14229, #13834, #13660
- For my own work, I always need to apply #13422, #13528, #14580, #12798 (which is slow during doctesting on Solaris and has no future of getting in), and the type1-fonts (which I haven't made into a proper patch),
- New features: #13246, #14580, #14112, #13576
(I just realized that two of them should have been set to needs review!)
comment:16 Changed 6 years ago by
- Merged in set to sage-5.10.rc0
- Resolution set to fixed
- Status changed from positive_review to closed
See also #13891, where some of the behavior there should be reverted a little.