From 95efa64cf89f19a5200a21471d58f31ff96a3c24 Mon Sep 17 00:00:00 2001 From: Alif Be Date: Thu, 7 May 2026 13:58:42 +0200 Subject: [PATCH] Prioritize kroend over krowend/krogend Treat NaN kroend as missing and fall back to legacy endpoints --- src/pyscal/factory.py | 74 +++++++++++++++++++--------- tests/test_factory.py | 112 +++++++++++++++++++++++++++++++++++++++--- 2 files changed, 154 insertions(+), 32 deletions(-) diff --git a/src/pyscal/factory.py b/src/pyscal/factory.py index 0657611b..9733652e 100644 --- a/src/pyscal/factory.py +++ b/src/pyscal/factory.py @@ -1424,9 +1424,11 @@ def check_deprecated(params: dict[str, Any]) -> None: def kro_endpoint_wo(params: dict[str, Any]) -> dict[str, Any]: """ Normalize parameters to be used for creating a WaterOil object: - - If 'krowend' is present, rename it to 'kroend'. - - If both 'krowend' and 'kroend' are provided, use the 'krowend' value - and log a warning. + - If only 'krowend' is present and not NaN, map it to 'kroend'. + - If both 'krowend' and 'kroend' are present, treat NaN as missing for + endpoint selection; when both are valid, legacy 'kroend' is prioritized. + - Log a warning only when both are present and both are not NaN. + - Always remove 'krowend'. - Always remove 'krogend' if present (not relevant for WaterOil). Args: @@ -1438,17 +1440,28 @@ def kro_endpoint_wo(params: dict[str, Any]) -> dict[str, Any]: params_copy = params.copy() if "krowend" in params_copy: + krowend = params_copy["krowend"] + krowend_valid = not pd.isna(krowend) if "kroend" in params_copy: - logger.warning( - "Both 'krowend'=%r and 'kroend'=%r were provided; using the " - "'krowend' value for WaterOil construction.", - params_copy["krowend"], - params_copy["kroend"], - ) - # Overwrite any existing 'kroend' with the 'krowend' value, then drop 'krowend' - params_copy["kroend"] = params_copy.pop("krowend") + kroend = params_copy["kroend"] + kroend_valid = not pd.isna(kroend) + + if kroend_valid and krowend_valid: + logger.warning( + "Both 'krowend'=%r and 'kroend'=%r were provided; using the " + "'kroend' value for WaterOil construction.", + krowend, + kroend, + ) + elif not kroend_valid and krowend_valid: + params_copy["kroend"] = krowend + elif krowend_valid: + # Rename 'krowend' to 'kroend' + params_copy["kroend"] = krowend + + # Drop 'krowend' to avoid confusion + params_copy.pop("krowend", None) - # Remove GasOil key if it sneaks in params_copy.pop("krogend", None) return params_copy @@ -1457,9 +1470,11 @@ def kro_endpoint_wo(params: dict[str, Any]) -> dict[str, Any]: def kro_endpoint_go(params: dict[str, Any]) -> dict[str, Any]: """ Normalize parameters to be used for creating a GasOil object: - - If 'krogend' is present, rename it to 'kroend'. - - If both 'krogend' and 'kroend' are provided, use the 'krogend' value - and log a warning. + - If only 'krogend' is present and not NaN, map it to 'kroend'. + - If both 'krogend' and 'kroend' are present, treat NaN as missing for + endpoint selection; when both are valid, legacy 'kroend' is prioritized. + - Log a warning only when both are present and both are not NaN. + - Always remove 'krogend'. - Always remove 'krowend' if present (not relevant for GasOil). Args: @@ -1471,17 +1486,28 @@ def kro_endpoint_go(params: dict[str, Any]) -> dict[str, Any]: params_copy = params.copy() if "krogend" in params_copy: + krogend = params_copy["krogend"] + krogend_valid = not pd.isna(krogend) if "kroend" in params_copy: - logger.warning( - "Both 'krogend'=%r and 'kroend'=%r were provided; using the " - "'krogend' value for GasOil construction.", - params_copy["krogend"], - params_copy["kroend"], - ) - # Overwrite any existing 'kroend' with the 'krowend' value, then drop 'krowend' - params_copy["kroend"] = params_copy.pop("krogend") + kroend = params_copy["kroend"] + kroend_valid = not pd.isna(kroend) + + if kroend_valid and krogend_valid: + logger.warning( + "Both 'krogend'=%r and 'kroend'=%r were provided; using the " + "'kroend' value for GasOil construction.", + krogend, + kroend, + ) + elif not kroend_valid and krogend_valid: + params_copy["kroend"] = krogend + elif krogend_valid: + # Rename 'krogend' to 'kroend' + params_copy["kroend"] = krogend + + # Drop 'krogend' to avoid confusion + params_copy.pop("krogend", None) - # Remove WaterOil key if it sneaks in params_copy.pop("krowend", None) return params_copy diff --git a/tests/test_factory.py b/tests/test_factory.py index d1389f24..e017157b 100644 --- a/tests/test_factory.py +++ b/tests/test_factory.py @@ -663,12 +663,10 @@ def test_factory_wateroilgas_krowgend(): """ Test normalize krowend and krogend to kroend and put into create_water_oil and create_oil_gas, respectively - Normalize WaterOil parameters to be used for creating a - WaterOil(look into swof) object: - If 'krowend' is present, rename it to 'kroend'. - - If both 'krowend' and 'kroend' are provided, use the 'krowend' value - and log a warning. - - Always remove 'krogend' if present (not relevant for WaterOil). + - If 'krogend' is present, rename it to 'kroend'. + - Always remove 'krowend' and 'krogend' if present + (not relevant for WaterOil/GasOil). """ wog = create_water_oil_gas( @@ -679,7 +677,6 @@ def test_factory_wateroilgas_krowgend(): "nog": 2.5, "krowend": 0.6, "krogend": 0.7, - "kroend": 0.5, } ) swof = wog.SWOF() @@ -692,7 +689,46 @@ def test_factory_wateroilgas_krowgend(): assert "Corey krw" in swof assert "Corey krow" in swof assert "kroend=0.6" in swof + assert "krowend" not in swof assert "krogend" not in swof + assert "krogend" not in sgof + assert "krowend" not in sgof + + check_table(wog.gasoil.table) + check_table(wog.wateroil.table) + + +def test_factory_swap_krowgend(): + """ + Test prioritize kroend over krowend and krogend. + Always remove 'krowend' and 'krogend' if present + (not relevant for WaterOil/GasOil). + """ + + wog = create_water_oil_gas( + { + "nw": 2, + "now": 3, + "ng": 1, + "nog": 2.5, + "krowend": 0.6, + "krogend": 0.7, + "kroend": 0.5, + } + ) + swof = wog.SWOF() + sgof = wog.SGOF() + sat_table_str_ok(swof) + sat_table_str_ok(sgof) + assert "Corey krg" in sgof + assert "Corey krog" in sgof + assert "kroend=0.5" in sgof + assert "Corey krw" in swof + assert "Corey krow" in swof + assert "kroend=0.5" in swof + assert "krowend" not in swof + assert "krogend" not in swof + assert "krogend" not in sgof assert "krowend" not in sgof check_table(wog.gasoil.table) @@ -717,15 +753,75 @@ def test_factory_wateroilgas_warning(caplog): ) assert ( "Both 'krowend'=0.6 and 'kroend'=0.5 were provided; " - "using the 'krowend' value for WaterOil construction." + "using the 'kroend' value for WaterOil construction." ) in caplog.text assert ( "Both 'krogend'=0.7 and 'kroend'=0.5 were provided; " - "using the 'krogend' value for GasOil construction." + "using the 'kroend' value for GasOil construction." ) in caplog.text +def test_factory_wateroilgas_nan_kroend(caplog): + """If kroend is NaN, valid phase-specific keys should be used without warning.""" + with caplog.at_level(logging.WARNING): + wog = create_water_oil_gas( + { + "nw": 2, + "now": 3, + "ng": 1, + "nog": 2.5, + "krowend": 0.6, + "krogend": 0.7, + "kroend": np.nan, + } + ) + + swof = wog.SWOF() + sgof = wog.SGOF() + sat_table_str_ok(swof) + sat_table_str_ok(sgof) + + assert "kroend=0.6" in swof + assert "kroend=0.7" in sgof + assert "krowend" not in swof + assert "krogend" not in swof + assert "krowend" not in sgof + assert "krogend" not in sgof + assert "Both 'krowend'" not in caplog.text + assert "Both 'krogend'" not in caplog.text + + +def test_factory_wateroilgas_nan_legacy_keeps_valid_kroend(caplog): + """If phase-specific keys are NaN, valid kroend should be kept without warning.""" + with caplog.at_level(logging.WARNING): + wog = create_water_oil_gas( + { + "nw": 2, + "now": 3, + "ng": 1, + "nog": 2.5, + "krowend": np.nan, + "krogend": np.nan, + "kroend": 0.5, + } + ) + + swof = wog.SWOF() + sgof = wog.SGOF() + sat_table_str_ok(swof) + sat_table_str_ok(sgof) + + assert "kroend=0.5" in swof + assert "kroend=0.5" in sgof + assert "krowend" not in swof + assert "krogend" not in swof + assert "krowend" not in sgof + assert "krogend" not in sgof + assert "Both 'krowend'" not in caplog.text + assert "Both 'krogend'" not in caplog.text + + def test_factory_wateroilgas_wo(): """Test making only wateroil through the wateroilgas factory""" wog = create_water_oil_gas(