Slightly tighten the Bbox/Transform API. #13110
Conversation
c8a178a to
306feb8
Compare
|
This is deprecating what was probably put in as a convenience function, but I see the logic of removing it. The tradeoff is that if the function is widely used, removing it is imposing a burden on users for a very small gain. |
|
I argued in #12059 that the transform API is complex and inconsistent, and would benefit from some tightening. |
306feb8 to
b186809
Compare
|
I'm not following the rationale behind removing this helper.... |
|
Do you know the difference between TransformedBbox, Transform.transform_bbox, and Bbox.transformed? They all do slightly different things (#12059), and I think it's too bad that this is not more obvious from the names of the APIs; in other words there are too many ways to "apply" a transform to a bbox, and I'm trying to start removing some of those. |
b186809 to
e4f7356
Compare
|
Agreed its a bit of a mess and cleanup would be great. Do you have a roadmap? Maybe make a "project" so the rest of us can help/comment/not do contradictory things? In the meantime though, this still seems like a useful helper 😉 |
|
It's basically discussed in #12059 -- still in progress. I would argue that |
|
Unfortunately, needs rebase again due to the broken imports. |
e4f7356 to
d739646
Compare
|
rebased |
d739646 to
db9df82
Compare
|
As a first step in simplifying the baffling transform API, I would like to see this go in. |
At least it's explicit that Bbox.inverse_transformed() follows the Bbox.transformed() semantics, not those of Transform.transform_bbox() or of TransformedBbox (sounds obvious, but you never know...).
|
rebased |
db9df82 to
39c9ac9
Compare
At least it's explicit that Bbox.inverse_transformed() follows the
Bbox.transformed() semantics, not those of Transform.transform_bbox() or
of TransformedBbox (#12059) (sounds obvious, but you never know...).
Also one less thing to implement for a possible future C implementation of the transform stack.
PR Summary
PR Checklist