From ae1ab6ab0e470047dee0ae89e1b01bc8d5b22384 Mon Sep 17 00:00:00 2001 From: mjreno Date: Wed, 1 Jul 2026 15:47:29 -0400 Subject: [PATCH 1/2] improve export signature --- autotest/test_binarygrid_util.py | 31 ++++++++++++++++++++++++++++++ flopy/mf6/utils/binarygrid_util.py | 29 +++++++++++++++++++++++++--- 2 files changed, 57 insertions(+), 3 deletions(-) diff --git a/autotest/test_binarygrid_util.py b/autotest/test_binarygrid_util.py index 6c88fd4c8..725aff491 100644 --- a/autotest/test_binarygrid_util.py +++ b/autotest/test_binarygrid_util.py @@ -629,3 +629,34 @@ def test_write_grb_disu_v1_upgrade_to_v2(tmp_path, mfgrd_test_path): np.testing.assert_array_equal(grb_new.ja, grb_orig.ja) np.testing.assert_allclose(grb_new.top, grb_orig.top) np.testing.assert_allclose(grb_new.bot, grb_orig.bot) + + +def test_write_grb_export_is_keyword_only(tmp_path, mfgrd_test_path): + """A positional call beyond filename raises.""" + grb = MfGrdFile(mfgrd_test_path / "nwtp3.dis.grb", verbose=False) + with pytest.raises(TypeError): + grb.export(tmp_path / "out.grb", "double", 1, "EPSG:26916", True) + + +def test_write_grb_empty_crs_argument_raises(tmp_path, mfgrd_test_path): + """An explicit empty/blank crs argument raises.""" + grb = MfGrdFile(mfgrd_test_path / "nwtp3.dis.grb", verbose=False) + with pytest.raises(ValueError, match="not a valid CRS"): + grb.export(tmp_path / "out.grb", crs="") + with pytest.raises(ValueError, match="not a valid CRS"): + grb.export(tmp_path / "out.grb", crs=" ") + + +def test_write_grb_blank_source_crs_treated_as_absent(tmp_path, mfgrd_test_path): + """A source file's blank CRS is treated as absent on re-export: + version drops to 1 and no CRS field is written.""" + grb = MfGrdFile(mfgrd_test_path / "flow_v2.dis.grb", verbose=False) + grb._datadict["CRS"] = "" + + output_file = tmp_path / "blank_crs.grb" + grb.export(output_file) + + grb_new = MfGrdFile(output_file, verbose=False) + assert grb_new.version == 1 + assert grb_new.crs is None + assert "CRS" not in grb_new._datadict diff --git a/flopy/mf6/utils/binarygrid_util.py b/flopy/mf6/utils/binarygrid_util.py index 360a4bdaa..13682e632 100644 --- a/flopy/mf6/utils/binarygrid_util.py +++ b/flopy/mf6/utils/binarygrid_util.py @@ -795,7 +795,15 @@ def cell2d(self): vertices, cell2d = None, None return vertices, cell2d - def export(self, filename, precision=None, version=None, crs=None, verbose=False): + def export( + self, + filename, + *, + precision=None, + version=None, + crs=None, + verbose=False, + ): """ Export the binary grid file to a new file. @@ -818,6 +826,11 @@ def export(self, filename, precision=None, version=None, crs=None, verbose=False verbose : bool, optional Print progress messages (default False) + All parameters except ``filename`` are keyword-only, so adding a + new parameter here in the future (e.g. a dedicated parameter and + property for a new GRB field, mirroring ``crs``/``.crs``) can + never silently change the meaning of an existing positional call. + Examples -------- >>> from flopy.mf6.utils import MfGrdFile @@ -832,10 +845,20 @@ def export(self, filename, precision=None, version=None, crs=None, verbose=False if precision is None: precision = self.precision + if isinstance(crs, str) and crs.strip() == "": + raise ValueError( + "crs='' is not a valid CRS. Omit the crs argument (or pass " + "crs=None) to leave the CRS unset." + ) + raw_crs = crs if crs is not None else self.crs effective_crs = _crs_to_string(raw_crs) if raw_crs is not None else None + if effective_crs is not None and effective_crs.strip() == "": + # A blank CRS carries no usable information. Treat it the + # same as no CRS rather than writing an empty crs string. + effective_crs = None if version is None: - version = 2 if effective_crs else 1 + version = 2 if effective_crs is not None else 1 float_type = "SINGLE" if precision.lower() == "single" else "DOUBLE" @@ -864,7 +887,7 @@ def export(self, filename, precision=None, version=None, crs=None, verbose=False data_dict[key] = self._datadict[key] if version >= 2: - if not effective_crs: + if effective_crs is None: raise ValueError( "version=2 requires a CRS string. Provide crs= or use a " "version 2 source file." From eff4582fc2107c011b0f40b0822f7d54fe33155c Mon Sep 17 00:00:00 2001 From: mjreno Date: Wed, 1 Jul 2026 17:56:46 -0400 Subject: [PATCH 2/2] cleanup --- autotest/test_binarygrid_util.py | 33 ++++++++++++++++++++++++------ flopy/mf6/utils/binarygrid_util.py | 5 +---- 2 files changed, 28 insertions(+), 10 deletions(-) diff --git a/autotest/test_binarygrid_util.py b/autotest/test_binarygrid_util.py index 725aff491..143710f09 100644 --- a/autotest/test_binarygrid_util.py +++ b/autotest/test_binarygrid_util.py @@ -638,13 +638,34 @@ def test_write_grb_export_is_keyword_only(tmp_path, mfgrd_test_path): grb.export(tmp_path / "out.grb", "double", 1, "EPSG:26916", True) -def test_write_grb_empty_crs_argument_raises(tmp_path, mfgrd_test_path): - """An explicit empty/blank crs argument raises.""" +def test_write_grb_empty_crs_argument_falls_back(tmp_path, mfgrd_test_path): + """An explicit empty/blank crs argument is treated like crs=None: it + falls back to the source file's CRS instead of overriding it.""" + grb = MfGrdFile(mfgrd_test_path / "flow_v2.dis.grb", verbose=False) + assert grb.crs is not None + + output_file = tmp_path / "out.grb" + grb.export(output_file, crs="") + grb_new = MfGrdFile(output_file, verbose=False) + assert grb_new.crs == grb.crs + + output_file2 = tmp_path / "out2.grb" + grb.export(output_file2, crs=" ") + grb_new2 = MfGrdFile(output_file2, verbose=False) + assert grb_new2.crs == grb.crs + + +def test_write_grb_empty_crs_argument_no_source_crs(tmp_path, mfgrd_test_path): + """An explicit empty/blank crs argument with no source CRS to fall + back on results in a version 1 file with no CRS, same as crs=None.""" grb = MfGrdFile(mfgrd_test_path / "nwtp3.dis.grb", verbose=False) - with pytest.raises(ValueError, match="not a valid CRS"): - grb.export(tmp_path / "out.grb", crs="") - with pytest.raises(ValueError, match="not a valid CRS"): - grb.export(tmp_path / "out.grb", crs=" ") + assert grb.crs is None + + output_file = tmp_path / "out.grb" + grb.export(output_file, crs="") + grb_new = MfGrdFile(output_file, verbose=False) + assert grb_new.version == 1 + assert grb_new.crs is None def test_write_grb_blank_source_crs_treated_as_absent(tmp_path, mfgrd_test_path): diff --git a/flopy/mf6/utils/binarygrid_util.py b/flopy/mf6/utils/binarygrid_util.py index 13682e632..88e9b565c 100644 --- a/flopy/mf6/utils/binarygrid_util.py +++ b/flopy/mf6/utils/binarygrid_util.py @@ -846,10 +846,7 @@ def export( precision = self.precision if isinstance(crs, str) and crs.strip() == "": - raise ValueError( - "crs='' is not a valid CRS. Omit the crs argument (or pass " - "crs=None) to leave the CRS unset." - ) + crs = None raw_crs = crs if crs is not None else self.crs effective_crs = _crs_to_string(raw_crs) if raw_crs is not None else None