Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Remove handling of boxsize as a unyt_array #21

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
13 changes: 2 additions & 11 deletions swiftgalaxy/reader.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
the :class:`SWIFTGalaxy` class.
"""

from warnings import warn, catch_warnings, filterwarnings
from warnings import warn
from copy import deepcopy
import numpy as np
from scipy.spatial.transform import Rotation
Expand Down Expand Up @@ -57,16 +57,7 @@ def _apply_box_wrap(coords: cosmo_array, boxsize: Optional[cosmo_array]) -> cosm
"""
if boxsize is None:
return coords
# Would like to ensure comoving coordinates here, but metadata gives boxsize as a
# unyt_array instead of a cosmo_array. Pending swiftsimio issue #128. When
# implementing, remove cast(s) to cosmo_array in test_coordinate_transformations.
with catch_warnings():
filterwarnings(
"ignore", category=RuntimeWarning, message="Mixing ufunc arguments"
)
retval = (coords + boxsize / 2.0) % boxsize - boxsize / 2.0
retval.comoving = coords.comoving
retval.cosmo_factor = coords.cosmo_factor
retval = (coords + boxsize / 2.0) % boxsize - boxsize / 2.0
return retval


Expand Down
27 changes: 1 addition & 26 deletions tests/test_coordinate_transformations.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import pytest
import numpy as np
import unyt as u
from warnings import warn
from unyt.testing import assert_allclose_units
from swiftsimio.objects import cosmo_array, cosmo_factor, a
from scipy.spatial.transform import Rotation
Expand Down Expand Up @@ -378,21 +377,8 @@ def test_box_wrap(self, sg, particle_name, coordinate_name):
"""
Check that translating by two box lengths wraps back to previous state.
"""
if hasattr(sg.metadata.boxsize, "comoving"):
# if this warning produced, remove cast to cosmo_array below
# and this warning block
warn(
"SWIFTSimIO is now giving boxsize as comso_array, update this test.",
category=RuntimeWarning,
)
else:
boxsize = cosmo_array(
sg.metadata.boxsize,
comoving=True,
cosmo_factor=cosmo_factor(a**1, scale_factor=1.0),
)
xyz_before = getattr(getattr(sg, particle_name), f"{coordinate_name}")
sg.translate(-2 * boxsize) # -2x box size
sg.translate(-2 * sg.metadata.boxsize) # -2x box size
xyz = getattr(getattr(sg, particle_name), f"{coordinate_name}")
assert_allclose_units(xyz_before, xyz, rtol=1.0e-4, atol=abstol_c)

Expand Down Expand Up @@ -544,14 +530,3 @@ def test_copied_velocity_transform(self, sg):
assert_allclose_units(
sg.gas.velocities, sg2.gas.velocities, rtol=1.0e-4, atol=abstol_v
)


@pytest.mark.xfail
class TestBoxsizeIsCosmoArray:
def test_boxsize_is_cosmo_array(self, sg):
# When swiftsimio issue #128 is resolved:
# - This test will unexpectedly pass.
# - Remove this test.
# - Remove catch_warnings and filterwarnings from _apply_box_wrap in reader.py
# - Remove explicit attribute copying in _apply_box_wrap in reader.py
assert hasattr(sg.metadata.boxsize, "comoving")