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

vector plot should have optional "start" argument #9962

Closed
jasongrout opened this issue Sep 21, 2010 · 27 comments
Closed

vector plot should have optional "start" argument #9962

jasongrout opened this issue Sep 21, 2010 · 27 comments

Comments

@jasongrout
Copy link
Member

It would be really nice if this plotted an arrow from (1,2) to (3,4):

sage: v=vector([1,2])
sage: u=vector([2,2])
sage: plot(u,start=v)

or maybe the option should be "base" or "origin"

To fix this, just change the plot method in devel/sage/sage/modules/free_module_element.pyx


Apply attachment: trac_9962_vector_start.2.patch and attachment: trac_9962-reviewer.patch

CC: @sagetrac-ryan @kcrisman

Component: graphics

Author: Ryan Grout, Karl-Dieter Crisman

Reviewer: Aly Deines, Karl-Dieter Crisman

Merged: sage-4.7.1.alpha2

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

@jasongrout
Copy link
Member Author

comment:1

I think "start=list/tuple/vector" is the best convention for this option.

@sagetrac-ryan
Copy link
Mannequin

sagetrac-ryan mannequin commented Jan 9, 2011

tentative patch

@sagetrac-ryan
Copy link
Mannequin

sagetrac-ryan mannequin commented Jan 9, 2011

comment:2

Attachment: trac_9962_start_vector.patch.gz

the patch gives the described output, but someone should double check my code for correctness.

@sagetrac-ryan sagetrac-ryan mannequin added the s: needs review label Jan 9, 2011
@adeines
Copy link
Mannequin

adeines mannequin commented Jan 9, 2011

comment:3

Replying to @sagetrac-ryan:

the patch gives the described output, but someone should double check my code for correctness.

It looks good to me.

@jdemeyer
Copy link

jdemeyer commented Jan 9, 2011

Author: Ryan Grout

@jdemeyer
Copy link

jdemeyer commented Jan 9, 2011

Reviewer: Aly Deines

@jdemeyer jdemeyer modified the milestones: sage-4.6.1, sage-4.6.2 Jan 9, 2011
@sagetrac-ryan
Copy link
Mannequin

sagetrac-ryan mannequin commented Jan 10, 2011

updated patch

@sagetrac-ryan
Copy link
Mannequin

sagetrac-ryan mannequin commented Jan 11, 2011

comment:5

Attachment: trac_9962_vector_start.patch.gz

@sagetrac-ryan
Copy link
Mannequin

sagetrac-ryan mannequin commented Jan 11, 2011

updated patch + doctests

@sagetrac-ryan
Copy link
Mannequin

sagetrac-ryan mannequin commented Jan 11, 2011

comment:6

Attachment: trac_9962_vector_start.2.patch.gz

updated patch:

Included error handling for some cases where the two coordinates are not of the same dimension. Also, added doctests.

Much cleaner patch.

I appreciate the suggestions for making this patch better.

@adeines
Copy link
Mannequin

adeines mannequin commented Jan 11, 2011

comment:8

Replying to @kcrisman:

@jdemeyer
Copy link

comment:9

Please make it clear which patches have to be applied.

@kcrisman
Copy link
Member

comment:11

You may want to check whether this really does the same thing as v.plot() - see #10638.

@jdemeyer
Copy link

comment:12

Replying to @kcrisman:

You may want to check whether this really does the same thing as v.plot() - see #10638.

I am interpreting this comment as a needs_work (if it's not, then please set positive_review).

@kcrisman
Copy link
Member

comment:13

No, it's neither positive review nor needs work. I haven't had the time to review this, but any reviewer should be sure to check this out. That's all I am saying.

@kcrisman
Copy link
Member

Changed reviewer from Aly Deines to Aly Deines, Karl-Dieter Crisman

@kcrisman
Copy link
Member

comment:14

This patch causes an amusing error, which does not occur in the vanilla Sage:


sage: plot(vector([1]))
ERROR: An unexpected error occurred while tokenizing input
The following traceback may be corrupted or invalid
The error message is: ('EOF in multi-line statement', (2, 0))

---------------------------------------------------------------------------
ArithmeticError                           Traceback (most recent call last)
<snip>
ArithmeticError: Cross product only defined for vectors of length three or seven, not (3 and 1)

I think I can fix this, so patch hopefully coming up.

@kcrisman
Copy link
Member

comment:15

I think I have it fixed. However, the bizarre error messages remain for one-dimensional arrows, so I have created followup ticket #10925 for that.

@kcrisman
Copy link
Member

Attachment: trac_9962-reviewer.patch.gz

Apply after vector_start.2 patch

@kcrisman
Copy link
Member

Changed author from Ryan Grout to Ryan Grout, Karl-Dieter Crisman

@kcrisman
Copy link
Member

comment:16

Okay, positive review on this nice addition from the original patch.

The reviewer patch still needs review; it fixes the problem by extending the one-dimensional start as well, and adds/spruces up some documentation. Reviewer should check things work, doctests, and that documentation looks ok.

@kcrisman

This comment has been minimized.

@kcrisman
Copy link
Member

comment:17

Ryan, you can totally review the reviewer patch. Just check that it still does what you wanted, that the doctests are correct and pass, and that ./sage -docbuild reference html looks nice in the documentation for free modules.

@sagetrac-ryan
Copy link
Mannequin

sagetrac-ryan mannequin commented May 13, 2011

comment:18

Replying to @kcrisman:

Ryan, you can totally review the reviewer patch. Just check that it still does what you wanted, that the doctests are correct and pass, and that ./sage -docbuild reference html looks nice in the documentation for free modules.

I'll look at it this evening.

@sagetrac-ryan
Copy link
Mannequin

sagetrac-ryan mannequin commented May 14, 2011

comment:19

everything looks good here. I'm going to go ahead an change this to positive review for the reviewer patch.

@jdemeyer
Copy link

Merged: sage-4.7.1.alpha2

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