[with patch, positive review] arrow3d((0,0,0), (0,0,-1)) plots an *upward* pointing arrow

[jason]

Try the following:

plot(vector((0,0,-1)))

The resulting vector points up, but should point down.

[jason]

The problem is in arrow3d:

sage: arrow3d((0,0,0), (0,0,-1))
*points up*

[jason]

All right, this was because we were taking the cross product with (0,0,1) and using that to decide whether or not to rotate the vector, ignoring the case that we point in the opposite direction (but still have a cross product of zero).

Patch will be attached. I'm marking this a blocker since it gives a *wrong* result and William said that wrong results should be marked as blockers.

[mabshoff]

Jason,

how does this ticket related to #1777? [also opened by you]

Cheers,

Michael

[jason]

It doesn't relate.

In this ticket, the arrow is actually wrong (the assumption in the code is that "parallel to (0,0,1)" == "pointing in the same direction as (0,0,1)", which is just wrong.

The issue in #1777 is a cosmetic issue related to the menus in JMOL.

[ddrake]

The patch works as advertised and the code looks good to me. Positive review for the code, and I'll give it full positive review once you sort out this minor complaint about the doctest: instead of "A downward pointing arrow should have a transformation scaling the points to their negatives" do you mean to say something like "a downward-pointing arrow corresponds to a transformation which reverses the z-coordinates of points"?

Mostly I'm confused because when you give me a vector in R³ and ask me what linear transformation of R³ to itself that it's describing, I naturally think of reflection across the perpendicular plane. The vector (0,0,-1) doesn't say "take (x,y,z) to (-x,-y,-z)" to me...but maybe this is just me, and perhaps it's more an issue with get_transformation than with arrow3d.

At any rate, if you're a bit more clear in that description, you'll be good to go.

[jason]

Arrows are always constructed pointing up in the z direction from the origin, and then rotated/translated into place. This works for every arrow direction except the -z direction. I take care of the issue by testing to see if the arrow should point in the -z direction, and if it should, just scaling the constructed arrow by -1 (i.e., every point is sent to its negative). The scaled arrow then points downwards. The doctest just tests that the scale of -1 is applied to the arrow. I'm not sure how to make it clearer.

[jason]

I put up a new patch with the above explanation above the doctest.

[mabshoff]

Looks good to me. I like the added explanation. Positive review.

Cheers,

Michael

[mabshoff]

Merged in Sage 3.0.alpha3