Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

make generic_graph.plot() pass its options to show #14632

Closed
ppurka opened this issue May 23, 2013 · 19 comments
Closed

make generic_graph.plot() pass its options to show #14632

ppurka opened this issue May 23, 2013 · 19 comments

Comments

@ppurka
Copy link
Member

ppurka commented May 23, 2013

Essentially what the title says. If it doesn't support the option, then there are two ways it can be handled:

  1. it will give an error from show just like the other plot functions do.
  2. 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: attachment: trac_14632-generic_graph_plot.patch

CC: @nathanncohen

Component: graph theory

Author: Punarbasu Purkayastha

Reviewer: Nathann Cohen

Merged: sage-5.10.rc0

Issue created by migration from https://trac.sagemath.org/ticket/14632

@williamstein
Copy link
Contributor

comment:1

See also #13891, where some of the behavior there should be reverted a little.

@ppurka

This comment has been minimized.

@ppurka
Copy link
Member Author

ppurka commented May 24, 2013

comment:2

Added a patch. Please check that none of your graph plots are broken :)

@nathanncohen
Copy link
Mannequin

nathanncohen mannequin commented May 24, 2013

comment:3

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

@ppurka
Copy link
Member Author

ppurka commented May 24, 2013

comment:4

Added a check for incorrect arguments. I check against graphplot_options and the show options that are already extracted.

@nathanncohen
Copy link
Mannequin

nathanncohen mannequin commented May 24, 2013

comment:6

Coooooooooooooooool ! Thank you very much :-)

Nathann

@nathanncohen
Copy link
Mannequin

nathanncohen mannequin commented May 24, 2013

Reviewer: Nathann Cohen

@nathanncohen
Copy link
Mannequin

nathanncohen mannequin commented May 24, 2013

Author: Punarbasu Purkayastha

@ppurka
Copy link
Member Author

ppurka commented May 25, 2013

comment:7

That was fast :-O Thanks!

@nathanncohen
Copy link
Mannequin

nathanncohen mannequin commented May 26, 2013

comment:8

Come on thepatch is 5kb long. Let's not bury those for months :-P

Nathann

@ppurka
Copy link
Member Author

ppurka commented May 26, 2013

Attachment: trac_14632-generic_graph_plot.patch.gz

Apply to devel/sage (Update with an additional one colon)

@ppurka
Copy link
Member Author

ppurka commented May 26, 2013

comment:9

Replying to @nathanncohen:

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.

@nathanncohen
Copy link
Mannequin

nathanncohen mannequin commented May 26, 2013

comment:10

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

@ppurka
Copy link
Member Author

ppurka commented May 26, 2013

comment:11

Replying to @nathanncohen:

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.

@nathanncohen
Copy link
Mannequin

nathanncohen mannequin commented May 26, 2013

comment:12

Oh. Do you have many patches waiting for a review ?

Nathann

@ppurka
Copy link
Member Author

ppurka commented May 26, 2013

comment:13

Replying to @nathanncohen:

Oh. Do you have many patches waiting for a review ?

Nathann

Yeah. Many of them. :-(

@nathanncohen
Copy link
Mannequin

nathanncohen mannequin commented May 26, 2013

comment:14

Yeah. Many of them. :-(

I'll regret that...

--> give me the numbers please :-P

Nathann_who_secretly_(actually_not)_hates_graphics

@ppurka
Copy link
Member Author

ppurka commented May 26, 2013

comment:15

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

  1. You can ignore the first six patches because the first two were being reviewed, while the next four will probably never get in.
  2. Trivial patch: legend marker 'o' is incorrectly printed #13690
  3. For improving consistency the tickets are Remove vacuous solutions from solve #14229, Clean up linestyle arguments throughout Sage #13834, Auto convert matrices over RR/CC to RDF/CDF for eigenvalue/eigenvector computations #13660
  4. For my own work, I always need to apply Log scale plots should auto-expand axes range to include at least 2 ticks #13422, ymin is not respected in logarithmic plots #13528, Set individual legend font color in Sage #14580, list_plot3d plots extraneous points at z=0 and doesn't take color or rgbcolor as keywords #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),
  5. New features: Automatic exclusion of non-domain points in things like arcsec #13246, Set individual legend font color in Sage #14580, Allow to turn off axes selectively in plot #14112, add options about custom markers to point2d #13576

(I just realized that two of them should have been set to needs review!)

@jdemeyer
Copy link

Merged: sage-5.10.rc0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants