-
Notifications
You must be signed in to change notification settings - Fork 4
Reset object API #47
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
Reset object API #47
Conversation
|
I must admit that I'd feel a bit sad to see this part of the API go. I now have some experience with the new (functional) pylinalg API in pygfx/pygfx#457. Overall the API is very clean and Pythonic and Numpy-ish (?), but it can also feel rather verbose. For example, to apply two extra rotations to a rotation: # With the functional API
rot2 = la.quaternion_multiply(
r_azimuth, la.quaternion_multiply(rot1, r_elevation)
)
# With an object API, this could be
rot2 = rot1 * r_azimuth * r_elevationOr this example to transform a vector with two matrices. # With the functional API
pos1 = la.vector_apply_matrix(
la.vector_apply_matrix((screen_dist, 0, center[2]), camera_projection_inverse),
camera_world,
)
# With an object API this could be
pos1 = (
Vector3(screen_dist, 0, center[2])
.transform(camera_projection_inverse)
.transform(camera_world)
)
# Or perhaps even
pos1 = (camera_world * camera_projection_inverse).apply((screen_dist, 0, center[2]))My point is that there is still value in an object API that exposes primitive linalg objects (points, directions, planes) and transformations (translation, rotation, scale). For one because it makes code easier to read, but also imagine that in an interactive session, you type "some_position.", and autocompletion shows you what you can do. I think that the API in #25, which is rather specific to a transform object for the pygfx WorldObject, is not strictly a replacement for what is thrown away in this PR. |
|
That said, building and maintaining an extra API might not be worth it, considering that the linalg code is restricted to cameras, controllers, the gizmo, and the WorldObject. (Also since for the latter we plan on a specific solution). |
almarklein
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With a bit of pain, but I approve 😉
I get that. We can reintroduce it later when we feel like it would be worthwhile.
Personally, I think the functional API is more readable here.
So, I've come to think that's generally not really the case.
Well, yes, there's value in that. On the other hand, it's also limiting in the sense that it's not a plain numpy array anymore, even though that is probably an expectation some of our users will have.
I agree with that! I just feel like working with plain ndarrays, the functional API, and eventually the Transform object, is already going to lead to a worthwhile improvement in pygfx. I don't think the object API we are removing here makes it that much better. So I agree with your statement that it's not worthwhile right now. Put another way, because we can work with plain ndarrays, we don't have to come up with a new abstraction, and that decreases code complexity and risk of "doing it wrong". |
This PR:
import pylinalg as platoimport pylinalg as lain the test suite@almarklein I'd like to merge this before you do a pylinalg release (for pygfx/pygfx#457)