From dec03d0351f07b0a10a6677b230ac3d37dcf8e32 Mon Sep 17 00:00:00 2001 From: Diem Phung Nguyen Duy Date: Thu, 14 Nov 2024 20:55:07 +0700 Subject: [PATCH 1/3] Raise ValueError for conflicting pH and H+ --- src/pyEQL/solution.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/pyEQL/solution.py b/src/pyEQL/solution.py index c4c39d52..afd2177b 100644 --- a/src/pyEQL/solution.py +++ b/src/pyEQL/solution.py @@ -249,6 +249,11 @@ def __init__( self._solutes = solutes if self._solutes is None: self._solutes = {} + + # if user specifies non-default pH AND has H+ in solutes + if self._pH != 7 and "H+" in self._solutes: + raise ValueError("Cannot specify both a non-default pH and H+ at the same time. Please provide only one.") + if isinstance(self._solutes, dict): for k, v in self._solutes.items(): self.add_solute(k, v) From 09dd8177911f2587d7b96acb889fbc94e01336ea Mon Sep 17 00:00:00 2001 From: Diem Phung Nguyen Duy Date: Fri, 15 Nov 2024 08:58:17 +0700 Subject: [PATCH 2/3] Move code to for loop and add standardize_formula check --- src/pyEQL/solution.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/pyEQL/solution.py b/src/pyEQL/solution.py index afd2177b..54e508c7 100644 --- a/src/pyEQL/solution.py +++ b/src/pyEQL/solution.py @@ -250,12 +250,11 @@ def __init__( if self._solutes is None: self._solutes = {} - # if user specifies non-default pH AND has H+ in solutes - if self._pH != 7 and "H+" in self._solutes: - raise ValueError("Cannot specify both a non-default pH and H+ at the same time. Please provide only one.") - if isinstance(self._solutes, dict): for k, v in self._solutes.items(): + # if user specifies non-default pH AND has H+ in solutes + if standardize_formula(k) == "H[+1]" and self._pH != 7: + raise ValueError("Cannot specify both a non-default pH and H+ at the same time. Please provide only one.") self.add_solute(k, v) elif isinstance(self._solutes, list): msg = ( From 79627397f57bef341642c5a5c605a9a3413dd79d Mon Sep 17 00:00:00 2001 From: Diem Phung Date: Wed, 11 Dec 2024 10:17:47 +0700 Subject: [PATCH 3/3] add another branch and tests for the conditional --- src/pyEQL/solution.py | 9 +++++++-- tests/test_solution.py | 10 +++++++++- 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/src/pyEQL/solution.py b/src/pyEQL/solution.py index 54e508c7..99563d36 100644 --- a/src/pyEQL/solution.py +++ b/src/pyEQL/solution.py @@ -253,8 +253,13 @@ def __init__( if isinstance(self._solutes, dict): for k, v in self._solutes.items(): # if user specifies non-default pH AND has H+ in solutes - if standardize_formula(k) == "H[+1]" and self._pH != 7: - raise ValueError("Cannot specify both a non-default pH and H+ at the same time. Please provide only one.") + if standardize_formula(k) == "H[+1]": + if self._pH != 7: + raise ValueError( + "Cannot specify both a non-default pH and H+ at the same time. Please provide only one." + ) + # if user specifies default pH AND has H+ in solutes + self.logger.warning(f"H[+1] = {v} found in solutes. Overriding default pH with this value.") self.add_solute(k, v) elif isinstance(self._solutes, list): msg = ( diff --git a/tests/test_solution.py b/tests/test_solution.py index bd410c7e..7060756f 100644 --- a/tests/test_solution.py +++ b/tests/test_solution.py @@ -7,6 +7,7 @@ """ import copy +import logging import os import platform @@ -175,13 +176,20 @@ def test_diffusion_transport(s1, s2): assert np.isclose(D2 / D1, 0.80, atol=1e-2) -def test_init_raises(): +def test_init_raises(caplog): with pytest.raises(ValueError, match="random is not a valid value"): Solution(engine="random") with pytest.raises(ValueError, match="Non-aqueous solvent detected"): Solution(solvent="D2O") with pytest.raises(ValueError, match="Multiple solvents"): Solution(solvent=["D2O", "MeOH"]) + with pytest.raises(ValueError, match="Cannot specify both a non-default pH and H+"): + Solution({"HCO3-": "1 mM", "CO3--": "1 mM", "H+": "1 mM"}, pH=10) + module_log = logging.getLogger("pyEQL") + with caplog.at_level(logging.WARNING, "pyEQL"): + Solution({"HCO3-": "1 mM", "CO3--": "1 mM", "H+": "1 mM"}, log_level="warning") + assert module_log.level == logging.WARNING + assert "WARNING" in caplog.text def test_init_engines():