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.

Sign up for GitHub

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

Set the bounding_box within the forward_transform property #532

Merged
merged 3 commits into from
Dec 5, 2024
Merged

Set the bounding_box within the forward_transform property #532

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e7f299f
Always set the bounding box on the forward transform.
WilliamJamieson Nov 21, 2024
1d12619
Update changes
WilliamJamieson Nov 26, 2024
81790ad
Merge branch 'master' into refactor/move_ft_bbox
nden Dec 2, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion CHANGES.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,13 @@

- Force ``bounding_box`` to always be returned as a ``F`` ordered box. [#522]

- Move the bounding box attachment to the forward transform property. [#532]

- Adjust ``world_to_array_index_values`` to round to integer coordinates as specified by APE 14. [#525]

- Add warning filter to asdf extension to prevent the ``bounding_box`` order warning for gwcs
objects originating from a file. [#526]


0.21.0 (2024-03-10)
-------------------

Expand Down
27 changes: 15 additions & 12 deletions gwcs/wcs.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -258,15 +258,20 @@ def set_transform(self, from_frame, to_frame, transform):
def forward_transform(self):
"""
Return the total forward transform - from input to output coordinate frame.

"""

if self._pipeline:
#return functools.reduce(lambda x, y: x | y, [step[1] for step in self._pipeline[: -1]])
return functools.reduce(lambda x, y: x | y, [step.transform for step in self._pipeline[:-1]])
else:
if not self._pipeline:
return None

transform = functools.reduce(lambda x, y: x | y, [step.transform for step in self._pipeline[:-1]])

if self.bounding_box is not None:
# Currently compound models do not attempt to combine individual model
# bounding boxes. Get the forward transform and assign the bounding_box to it
# before evaluating it. The order Model.bounding_box is reversed.
transform.bounding_box = self.bounding_box
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you remind me why is it not necessary to reverse the bounding box when assigning to a transform? Is the default in modeling now order=F?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

gwcs/gwcs/wcs.py

Lines 1282 to 1312 in c01c6d0

def bounding_box(self):
"""
Return the range of acceptable values for each input axis.
The order of the axes is `~gwcs.coordinate_frames.CoordinateFrame.axes_order`.
"""
frames = self.available_frames
transform_0 = self.get_transform(frames[0], frames[1])
try:
bb = transform_0.bounding_box
except NotImplementedError:
return None
if (
# Check that the bounding_box was set on the instance (not a default)
transform_0._user_bounding_box is not None
# Check the order of that bounding_box is C
and bb.order == "C"
# Check that the bounding_box is not a single value
and (isinstance(bb, CompoundBoundingBox) or len(bb) > 1)
):
warnings.warn(
"The bounding_box was set in C order on the transform prior to being used in the gwcs!\n"
"Check that you indended that ordering for the bounding_box, and consider setting it in F order.\n"
"The bounding_box will remain meaning the same but will be converted to F order for consistency in the GWCS.",
GwcsBoundingBoxWarning
)
self.bounding_box = bb.bounding_box(order="F")
bb = self.bounding_box
return bb

Returns a astropy.modeling.bounding_box.ModelBoundingBox not a tuple. That full object will be processed correctly by astropy.


return transform

@property
def backward_transform(self):
"""
Expand Down Expand Up @@ -354,12 +359,6 @@ def __call__(self, *args, **kwargs):
if 'fill_value' not in kwargs:
kwargs['fill_value'] = np.nan

if self.bounding_box is not None:
# Currently compound models do not attempt to combine individual model
# bounding boxes. Get the forward transform and assign the bounding_box to it
# before evaluating it. The order Model.bounding_box is reversed.
transform.bounding_box = self.bounding_box

result = transform(*args, **kwargs)

if with_units:
Expand Down Expand Up @@ -1287,6 +1286,10 @@ def bounding_box(self):

frames = self.available_frames
transform_0 = self.get_transform(frames[0], frames[1])

if transform_0 is None:
return None

try:
bb = transform_0.bounding_box
except NotImplementedError:
Expand Down
Loading