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

Allow the exclusion of points from the plot range #6878

Closed
sagetrac-whuss mannequin opened this issue Sep 3, 2009 · 10 comments
Closed

Allow the exclusion of points from the plot range #6878

sagetrac-whuss mannequin opened this issue Sep 3, 2009 · 10 comments

Comments

@sagetrac-whuss
Copy link
Mannequin

sagetrac-whuss mannequin commented Sep 3, 2009

The attached patch adds a new option 'exclude' to the plot command
which allows to exclude points from the plot.

This is useful if there are discontinuities in the function you are plotting.

sage: plot(floor(x), (x, 1, 10), exclude = [1..10])

Component: graphics

Keywords: plot

Author: Wilfried Huss

Reviewer: Jason Grout, Ross Kyprianou

Merged: sage-4.3.3.alpha0

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

@sagetrac-whuss sagetrac-whuss mannequin added this to the sage-4.3.3 milestone Sep 3, 2009
@sagetrac-whuss sagetrac-whuss mannequin added the s: needs review label Sep 3, 2009
@jasongrout
Copy link
Member

comment:1

Very nice!

This line:

points = [e.right() for e in exclude.solve(v) if e.left() == v] 

should also check that v is not in the right side. Alternatively, you could use the solution_dict parameter to make sure you get a solution.

@sagetrac-whuss
Copy link
Mannequin Author

sagetrac-whuss mannequin commented Sep 28, 2009

comment:2

Replying to @jasongrout:

Very nice!

This line:

points = [e.right() for e in exclude.solve(v) if e.left() == v] 

should also check that v is not in the right side. Alternatively, you could use the solution_dict parameter to make sure you get a solution.

The new version of the patch includes your suggestion.

@jasongrout
Copy link
Member

comment:3

Some more comments after examining things more carefully:

  • Use "is not None" rather than "!= None", as the "is not None" is way, way faster (it's a pointer comparison)

  • If I have just a few excluded points, but lots more poles, won't the break during the calculation of the exclusions also jump out of the loop that deals with detecting poles? That means my poles past the last exclusion point are ignored.

  • Can you put a brief comment in about (x0 < exclusion_point <= x1 or x0 <= exclusion_point < x1)? It may be just that it's too late, but this is confusing me a bit.

Those are the only issues I find; the code other than that works fine. I have not tested the output (nice doctests, though!).

@sagetrac-whuss
Copy link
Mannequin Author

sagetrac-whuss mannequin commented Oct 16, 2009

comment:4

Attachment: trac_6878_exclude.patch.gz

Replying to @jasongrout:

Some more comments after examining things more carefully:

  • Use "is not None" rather than "!= None", as the "is not None" is way, way faster (it's a pointer comparison)

Done

  • If I have just a few excluded points, but lots more poles, won't the break during the calculation of the exclusions also jump out of the loop that deals with detecting poles? That means my poles past the last exclusion point are ignored.

You are right. I have fixed this.

  • Can you put a brief comment in about (x0 < exclusion_point <= x1 or x0 <= exclusion_point < x1)? It may be just that it's too late, but this is confusing me a bit.

I don't know anymore why I have written it that way. I have changed it to

(x0 <= exclusion_point <= x1)

Those are the only issues I find; the code other than that works fine. I have not tested the output (nice doctests, though!).

@sagetrac-whuss sagetrac-whuss mannequin assigned sagetrac-whuss and unassigned williamstein Dec 20, 2009
@sagetrac-rossk
Copy link
Mannequin

sagetrac-rossk mannequin commented Jan 31, 2010

comment:7

Works as advertised with the caveat that if the exclusion points are less than xmin or greater than xmax then the plot range is extended (beyond either xmin and xmax). Statements below demonstrate this.

(IMHO, I think this is new functionality that works and its easy to specify an exclude range that is inside xmin..xmax to get the plot range you want so it should go in the next milestone release - so Im giving it a positive review).

sage: plot(floor(x), xmin=0, xmax=10)              

sage: plot(floor(x), xmin=0, xmax=10, exclude = [1..15])

sage: plot(floor(x), xmin=0, xmax=10, exclude = [1..10])

sage: plot(floor(x), xmin=0, xmax=10, exclude = [5..10])

(I guess if we dont want the exclusion points to modify the plot range - which is ideal - this should be in a new ticket)

@qed777
Copy link
Mannequin

qed777 mannequin commented Feb 10, 2010

comment:8

The commit string is not sufficiently descriptive. I've refreshed it to

#6878: Allow the exclusion of points from the plot range

in the queue for 4.3.3.alpha0.

@qed777
Copy link
Mannequin

qed777 mannequin commented Feb 11, 2010

Reviewer: Jason Grout, Ross Kyprianou

@qed777
Copy link
Mannequin

qed777 mannequin commented Feb 11, 2010

comment:9

Please remember to update the relevant ticket fields --- the release managers use an automated script to generate lists of merged tickets.

@qed777
Copy link
Mannequin

qed777 mannequin commented Feb 11, 2010

Changed author from whuss to Wilfried Huss

@qed777 qed777 mannequin changed the title allow the exclusion of points from the plot range Allow the exclusion of points from the plot range Feb 11, 2010
@qed777
Copy link
Mannequin

qed777 mannequin commented Feb 11, 2010

Merged: sage-4.3.3.alpha0

@qed777 qed777 mannequin removed the s: positive review label Feb 11, 2010
@qed777 qed777 mannequin closed this as completed Feb 11, 2010
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

2 participants