From 03144f3c773a9d0431c72addf5b7556037a5966c Mon Sep 17 00:00:00 2001 From: chrishavlin Date: Tue, 31 Jan 2023 10:03:57 -0600 Subject: [PATCH] revert to changing how unit_system_name is saved --- yt/frontends/ytdata/tests/test_data_reload.py | 22 +++++++++++-------- yt/frontends/ytdata/utilities.py | 7 +++++- yt/units/unit_systems.py | 10 +-------- 3 files changed, 20 insertions(+), 19 deletions(-) diff --git a/yt/frontends/ytdata/tests/test_data_reload.py b/yt/frontends/ytdata/tests/test_data_reload.py index b93e5f4212a..33890e70325 100644 --- a/yt/frontends/ytdata/tests/test_data_reload.py +++ b/yt/frontends/ytdata/tests/test_data_reload.py @@ -1,23 +1,27 @@ import numpy as np -from yt.loaders import load, load_uniform_grid +from yt.loaders import load_uniform_grid +from yt.testing import requires_module_pytest +from yt.utilities.on_demand_imports import _h5py as h5py +@requires_module_pytest("h5py") def test_save_as_data_unit_system(tmp_path): - # This test loads in a DataContainer saved with save_as_dataset and checks - # for the "code" string in the unit_system name. Added to help - # avoid regressions with https://github.com/yt-project/yt/issues/4315 The - # test here is not a complete test as that bug manifests when restarting - # the python kernel. + # This test checks that the file saved with calling save_as_dataset + # for a ds with a "code" unit system contains the proper "unit_system_name". + # It checks the hdf5 file directly rather than using yt.load(), because + # https://github.com/yt-project/yt/issues/4315 only manifested restarting + # the python kernel (because the unit registry is state dependent). fi = tmp_path / "output_data.h5" shp = (4, 4, 4) data = {"density": np.random.random(shp)} ds = load_uniform_grid(data, shp, unit_system="code") - assert "code" in ds.unit_system.name + assert "code" in ds._unit_system_name + sp = ds.sphere(ds.domain_center, ds.domain_width[0] / 2.0) sp.save_as_dataset(fi) - ds_new = load(fi, unit_system="code") - assert "code" in ds_new.unit_system.name + with h5py.File(fi, mode="r") as f: + assert f.attrs["unit_system_name"] == "code" diff --git a/yt/frontends/ytdata/utilities.py b/yt/frontends/ytdata/utilities.py index 90454c7ffac..557cb8acadb 100644 --- a/yt/frontends/ytdata/utilities.py +++ b/yt/frontends/ytdata/utilities.py @@ -103,7 +103,12 @@ def save_as_dataset(ds, filename, data, field_types=None, extra_attrs=None): _yt_array_hdf5_attr(fh, "unit_registry_json", ds.unit_registry.to_json()) if hasattr(ds, "unit_system"): - _yt_array_hdf5_attr(fh, "unit_system_name", ds.unit_system.name.split("_")[0]) + # Note: ds._unit_system_name is written here rather than + # ds.unit_system.name because for a 'code' unit system, ds.unit_system.name + # is a hash, not a unit system. And on re-load, we want to designate + # a unit system not a hash value. + # See https://github.com/yt-project/yt/issues/4315 for more background. + _yt_array_hdf5_attr(fh, "unit_system_name", ds._unit_system_name) for attr in base_attrs: if isinstance(ds, dict): diff --git a/yt/units/unit_systems.py b/yt/units/unit_systems.py index 807a9c1351f..dcab8eb9c94 100644 --- a/yt/units/unit_systems.py +++ b/yt/units/unit_systems.py @@ -2,16 +2,8 @@ def create_code_unit_system(unit_registry, current_mks_unit=None): - - unit_system_name = "code_{}".format(unit_registry.unit_system_id) - # Note: the above unit system name needs both "code_" and the unique - # unit registry hash so that the unit system can be uniquely tied to - # a dataset. When calling `save_as_dataset`, only the "code" portion is - # saved as this name is supplied to Dataset._assign_unit_system(). - # See https://github.com/yt-project/yt/issues/4315 for more background. - code_unit_system = UnitSystem( - name=unit_system_name, + name=unit_registry.unit_system_id, length_unit="code_length", mass_unit="code_mass", time_unit="code_time",