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

add options about custom markers to point2d #13576

Closed
ppurka opened this issue Oct 7, 2012 · 49 comments
Closed

add options about custom markers to point2d #13576

ppurka opened this issue Oct 7, 2012 · 49 comments

Comments

@ppurka
Copy link
Member

ppurka commented Oct 7, 2012

point2d by default doesn't allow customization of markers. The patch below adds this functionality by adding two extra options marker and markeredgecolor.


Apply the git branch.

CC: @jasongrout @eviatarbach

Component: graphics

Author: Punarbasu Purkayastha

Branch/Commit: d7d7809

Reviewer: Karl-Dieter Crisman

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

@ppurka
Copy link
Member Author

ppurka commented Oct 7, 2012

Attachment: trac_13576-list_plot_doc.patch.gz

Apply to devel/sage

@ppurka
Copy link
Member Author

ppurka commented Oct 7, 2012

Author: Punarbasu Purkayastha

@ppurka

This comment has been minimized.

@ppurka
Copy link
Member Author

ppurka commented Oct 7, 2012

Work Issues: add customization options to point

@ppurka

This comment has been minimized.

@ppurka
Copy link
Member Author

ppurka commented Oct 7, 2012

Changed work issues from add customization options to point to none

@ppurka

This comment has been minimized.

@ppurka ppurka changed the title add documentation to list_plot about custom markers add options about custom markers to point2d Oct 7, 2012
@ppurka
Copy link
Member Author

ppurka commented Oct 7, 2012

Attachment: trac_13576-add_options_to_points.patch.gz

Apply to devel/sage

@ppurka

This comment has been minimized.

@kcrisman
Copy link
Member

comment:5

Oh, ppurka, I would love you if this works, I always get stuck using line(... linestyle='') for this very thing!

Is there an option for the width of the marker edge? They kind of overwhelm the marker color with the default point size; I have to go to size=60 or so to have it really seem like an edge.

Am I correct in my suspicion that markeredgecolor sort of automatically implies faceted=True in mpl internally? At least, the plots look like it, and faceted=False does nothing when I use markeredgecolor. The markeredgecolor seems to govern the color of the facet; I assume this is right, but the quite different names might make one think they control two different things.

In general I really like this patch, though, including the careful small local improvements that have nothing to do with the ticket but which are still valuable. Passes tests, so... answer my questions and you might get a positive review. Thanks for looking into this!

@ppurka
Copy link
Member Author

ppurka commented Dec 13, 2012

comment:6

I will need some time to recall what this patch is doing, and what other changes I had in mind regarding this.

I do remember that I resisted adding some extra keywords which are synonymous to the keywords in plot, list_plot, etc, and which have the same functionality as other keywords in this function. I think size is the same as markersize, color keyword is missing, markeredgecolor and faceted control the same thing, except the second is a boolean, and so on. Essentially, a bunch of inconsistencies on the Sage interface side mainly because the keywords used are the same as the mpl scatter command. It is really jarring that many different plot commands have different keywords for achieving the same effect.

Do you think it will be useful to either deprecate some keywords or add other keywords like color, markersize, etc which are similar to the keywords used in plot, list_plot, line, etc?

@kcrisman
Copy link
Member

Reviewer: Karl-Dieter Crisman

@kcrisman
Copy link
Member

comment:7

Do you think it will be useful to either deprecate some keywords or add other keywords like color, markersize, etc which are similar to the keywords used in plot, list_plot, line, etc?

Maybe. I wonder whether Jason might have some comments.

@jasongrout
Copy link
Member

comment:8

I actually just added marker options to points the other day in my personal copy of Sage so I could do some plots that I wanted to do :).

I think someone ought to go through each plotting function and make a comprehensive sweep to clean up options and make things more consistent. Right now, it's very confusing.

@ppurka
Copy link
Member Author

ppurka commented Dec 13, 2012

comment:9

Ok then, let's defer the cleanup to a different ticket - #13828 to be precise.

@ppurka
Copy link
Member Author

ppurka commented Dec 13, 2012

comment:10

Replying to @kcrisman:

Oh, ppurka, I would love you if this works, I always get stuck using line(... linestyle='') for this very thing!

Actually, it is true that line(... linestyle='') is more powerful than point (mpl's scatter). Many options are not present in the scatter command.

Is there an option for the width of the marker edge? They kind of overwhelm the marker color with the default point size; I have to go to size=60 or so to have it really seem like an edge.

There is no such option for scatter/point. But there is the option markeredgewidth for plot/line.

@kcrisman
Copy link
Member

comment:11

What I am hearing here is that there is no point in using mol's scatter instead of whatever we use for line2d. What does scatter have (speed, flexibility, something else) that mpl lines.Iine2D doesn't have?

@ppurka
Copy link
Member Author

ppurka commented Dec 14, 2012

comment:12

Replying to @kcrisman:

What I am hearing here is that there is no point in using mol's scatter instead of whatever we use for line2d. What does scatter have (speed, flexibility, something else) that mpl lines.Iine2D doesn't have?

Right. This could be best answered by mpl. Other than the cmap argument, which we don't use in point2d, I don't see how scatter is better than line2d.

Also, #13830 is relevant for any cleanup.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 25, 2014

Branch pushed to git repo; I updated commit sha1. New commits:

7542143add additional marker information to plot()

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 25, 2014

Changed commit from 9305db7 to 7542143

@ppurka
Copy link
Member Author

ppurka commented Jan 25, 2014

Changed work issues from marker, check list of markers to none

@ppurka
Copy link
Member Author

ppurka commented Jan 25, 2014

comment:30
  1. The information about marker was already present in points. The documentation asks users to look at points.option to find the information about allowed options. points.option does mention marker and was already asking the reader to look at plot for more information.

  2. The marker list in plot was mostly complete. Added a few more.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 25, 2014

Branch pushed to git repo; I updated commit sha1. New commits:

1493a78fix documentation for scatter_plot and points

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 25, 2014

Changed commit from 7542143 to 1493a78

@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.1, sage-6.2 Jan 30, 2014
@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.2, sage-6.3 May 6, 2014
@rwst
Copy link

rwst commented May 9, 2014

Work Issues: rebase

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 10, 2014

Changed commit from 1493a78 to d7d7809

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 10, 2014

Branch pushed to git repo; I updated commit sha1. New commits:

d7d7809merge develop (sage-6.2) on to current ticket

@ppurka
Copy link
Member Author

ppurka commented May 10, 2014

comment:36

Thanks for checking the ticket. The changes from develop have been merged on to current ticket.

@ppurka
Copy link
Member Author

ppurka commented May 10, 2014

Changed work issues from rebase to none

@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.3, sage-6.4 Aug 10, 2014
@kcrisman
Copy link
Member

comment:38

Eventually we'll need to unify scatter plot and point, I think there is even a ticket for this...

I would love to add more examples sometime.

sage: plot(x^2,marker="$x^2$",markeredgecolor='red')

Hilarious!

Anyway, I'm happy with this.

@vbraun
Copy link
Member

vbraun commented Aug 16, 2014

Changed branch from u/ppurka/ticket/13576 to d7d7809

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

8 participants