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

line3d does not take a tuple of points #9088

Closed
jasongrout opened this issue May 29, 2010 · 7 comments
Closed

line3d does not take a tuple of points #9088

jasongrout opened this issue May 29, 2010 · 7 comments

Comments

@jasongrout
Copy link
Member

Right now, this fails:

line3d(( (0,0,0), (1,2,3) ))

since the copy of the input data is not converted to a list. This is an easy fix.

CC: @kcrisman @sagetrac-mhampton

Component: graphics

Author: Jason Grout

Reviewer: Karl-Dieter Crisman

Merged: sage-4.5.2.alpha0

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

@jasongrout
Copy link
Member Author

Attachment: trac-9088-line3d-list-copy.2.patch.gz

@jasongrout
Copy link
Member Author

comment:1

Fix attached. This should be a trivial review!

@kcrisman
Copy link
Member

comment:2

Positive review.

@kcrisman
Copy link
Member

Reviewer: Karl-Dieter Crisman

@loefflerd
Copy link
Mannequin

loefflerd mannequin commented Jul 1, 2010

comment:3

This patch and #9066 seem to conflict -- if I apply both, I get a doctest failure:

sage -t  "devel/sage-reviewing/sage/plot/plot3d/shapes2.py"
**********************************************************************
File "/storage/masiao/sage-4.5.alpha1/devel/sage-reviewing/sage/plot/plot3d/shapes2.py", line 700:
    sage: P.tachyon_repr(P.default_render_params())
Expected:
    'Sphere center 1.0 2.0 3.0 Rad 0.015 texture84'
Got:
    'Sphere center 1.0 2.0 3.0 Rad 0.015 texture85'
**********************************************************************
File "/storage/masiao/sage-4.5.alpha1/devel/sage-reviewing/sage/plot/plot3d/shapes2.py", line 717:
    sage: P.obj_repr(P.default_render_params())[0][0:2]
Expected:
    ['g obj_1', 'usemtl texture86']
Got:
    ['g obj_1', 'usemtl texture87']
**********************************************************************
File "/storage/masiao/sage-4.5.alpha1/devel/sage-reviewing/sage/plot/plot3d/shapes2.py", line 825:
    sage: L.tachyon_repr(L.default_render_params())[0]
Expected:
    'FCylinder base 1.0 0.0 0.0 apex 0.999950000417 0.00999983333417 0.0001 rad 0.005 texture126'
Got:
    'FCylinder base 1.0 0.0 0.0 apex 0.999950000417 0.00999983333417 0.0001 rad 0.005 texture127'
**********************************************************************
3 items had failures:
   1 of   4 in __main__.example_13
   1 of   4 in __main__.example_14
   1 of   4 in __main__.example_19
***Test Failed*** 3 failures.

This doesn't come up if I apply either one of the patches on its own.

@kcrisman
Copy link
Member

kcrisman commented Jul 1, 2010

comment:4

I'm not exactly sure how the texture numbers get decided, but I think there is some linear order involved with their names. Anyway, this is easy enough to fix. We should definitely apply #9088 first because it is much more annoying, and then it would be very simple to add a reviewer patch to fix these trivialities.

I'll leave this as positive review until the release manager decides what order to merge these in, though - wouldn't want to overstep his prerogative :) then he can mark whichever one needs work thus.

@qed777
Copy link
Mannequin

qed777 mannequin commented Jul 20, 2010

Merged: sage-4.5.2.alpha0

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

3 participants