From b4649e6e1132dddd4611f6ae8d829f58e5ff03a1 Mon Sep 17 00:00:00 2001 From: Ryan Kingsbury Date: Mon, 4 Mar 2024 16:32:08 -0500 Subject: [PATCH 1/6] CI: add Apple M1 runner to github actions --- .github/workflows/testing.yaml | 32 +++++++++++++++++++------------- CHANGELOG.md | 17 +++++++++++++++-- src/pyEQL/solution.py | 2 +- tests/test_activity.py | 2 ++ tests/test_functions.py | 3 +++ tests/test_phreeqc.py | 4 ++++ tests/test_salt_matching.py | 3 +++ tests/test_solution.py | 5 +++++ 8 files changed, 52 insertions(+), 16 deletions(-) diff --git a/.github/workflows/testing.yaml b/.github/workflows/testing.yaml index 231596dd..2cda3225 100644 --- a/.github/workflows/testing.yaml +++ b/.github/workflows/testing.yaml @@ -1,17 +1,12 @@ name: testing on: - push: + pull_request: branches: - main paths-ignore: - - 'docs/' - CHANGELOG.md - pull_request: - branches: - - main - concurrency: group: ${{ github.workflow }}-${{ github.ref }} cancel-in-progress: true @@ -40,18 +35,29 @@ jobs: strategy: max-parallel: 6 matrix: - python-version: ["3.9", "3.10", "3.11", "3.12"] - platform: - - ubuntu-latest - - macos-latest - - windows-latest - runs-on: ubuntu-latest + # for most PRs, test the min and max supported python on every platform, test all python on ubuntu + python-version: ["3.9", "3.12"] + os: + - ubuntu-latest + - macos-latest + - macos-14 + - windows-latest + include: + - os: ubuntu-latest + python-version: "3.10" + - os: ubuntu-latest + python-version: "3.11" + # no python 3.9 on the macos-14 runner + exclude: + - os: macos-14 + python-version: "3.9" + runs-on: ${{ matrix.os }} steps: - uses: actions/checkout@v4 - name: Setup Python uses: actions/setup-python@v5 with: - python-version: ${{ matrix.python }}${{ matrix.dev }} + python-version: ${{ matrix.python-version }}${{ matrix.dev }} - name: Install test requirements run: | python -m pip install --upgrade pip diff --git a/CHANGELOG.md b/CHANGELOG.md index 1e953cbb..7c40ce34 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,15 +7,28 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## Unreleased -## [0.13.0] - 2024-03-05 +### Added + +- CI: Added Apple M1 runner (GitHub: `macos-14`) to the CI tests. ### Fixed -- `equilibrium.alpha()`: Fixed incorrect calculation of acid-base distribution coefficient for multiprotic acids. +- CI: Addressed several issues in the testing configuration which had resulted in testing + fewer operating systems x python version combinations than intended. CI tests now + correctly and comprehensively test every supported version of python on every os + (macos, windows, ubuntu). +- `utils.FormulaDict`: implemented `__contains__` so that `get()` works correctly in + python 3.12+. See https://github.com/python/cpython/issues/105524 - Docs: fixed many small problems in documentation causing equations and examples to render incorrectly. - `Solution.from_file`: Add missing `@classmethod` decorator; update documentation. +## [0.13.0] - 2024-03-05 + +### Fixed + +- `equilibrium.alpha()`: Fixed incorrect calculation of acid-base distribution coefficient for multiprotic acids. + ## [0.12.2] - 2024-02-25 ### Fixed diff --git a/src/pyEQL/solution.py b/src/pyEQL/solution.py index 6cff6c31..64aa2826 100644 --- a/src/pyEQL/solution.py +++ b/src/pyEQL/solution.py @@ -169,7 +169,7 @@ def __init__( if database is None: # load the default database, which is a JSONStore db_store = IonDB - elif isinstance(database, str | Path): + elif isinstance(database, (str, Path)): db_store = JSONStore(str(database), key="formula") logger.info(f"Created maggma JSONStore from .json file {database}") else: diff --git a/tests/test_activity.py b/tests/test_activity.py index a4dc460e..2f58cc8d 100644 --- a/tests/test_activity.py +++ b/tests/test_activity.py @@ -10,6 +10,7 @@ cases, the output is also tested against a well-established model published by USGS(PHREEQC) """ +import platform import numpy as np import pytest @@ -280,6 +281,7 @@ def test_activity_crc_rbcl(): assert np.isclose(result, expected, rtol=0.05) +@pytest.mark.skipif(platform.machine() == "arm64" and platform.system() == "Darwin", reason="arm64 not supported") def test_activity_crc_MgCl2(): """ calculate the activity coefficient of MgCl2 at each concentration and compare diff --git a/tests/test_functions.py b/tests/test_functions.py index 6816b212..dae0c814 100644 --- a/tests/test_functions.py +++ b/tests/test_functions.py @@ -2,6 +2,8 @@ Tests of pyEQL.functions module """ +import platform + import numpy as np import pytest @@ -39,6 +41,7 @@ def s2_i(): return Solution({"Na+": "1 mol/L", "Cl-": "1 mol/L"}, volume="10 L", engine="ideal") +@pytest.mark.skipif(platform.machine() == "arm64" and platform.system() == "Darwin", reason="arm64 not supported") def test_mixing_functions(s1, s2, s1_p, s2_p, s1_i, s2_i): # mixing energy and entropy of any solution with itself should be zero assert np.isclose(gibbs_mix(s1, s1).magnitude, 0) diff --git a/tests/test_phreeqc.py b/tests/test_phreeqc.py index bf6942d1..20850dce 100644 --- a/tests/test_phreeqc.py +++ b/tests/test_phreeqc.py @@ -7,6 +7,7 @@ """ import logging +import platform import numpy as np import pytest @@ -14,6 +15,9 @@ from pyEQL import Solution from pyEQL.engines import PhreeqcEOS +if platform.machine().startswith("arm64") and platform.system().startswith("Darwin"): + pytest.skip("skipping PHREEQC tests because arm64 is not supported", allow_module_level=True) + @pytest.fixture() def s1(): diff --git a/tests/test_salt_matching.py b/tests/test_salt_matching.py index c72f01b0..c1ea5fd9 100644 --- a/tests/test_salt_matching.py +++ b/tests/test_salt_matching.py @@ -5,8 +5,10 @@ This file contains tests for the salt-matching algorithm used by pyEQL in salt_ion_match.py """ +import platform import numpy as np +import pytest import pyEQL from pyEQL.salt_ion_match import Salt @@ -97,6 +99,7 @@ def test_single_ion(): assert s1.get_salt().nu_anion == 3 +@pytest.mark.skipif(platform.machine() == "arm64" and platform.system() == "Darwin", reason="arm64 not supported") def test_salt_with_equilibration(): """ test matching a solution containing a salt, before and after equilibration. diff --git a/tests/test_solution.py b/tests/test_solution.py index 9d0754e0..9c5a7eb2 100644 --- a/tests/test_solution.py +++ b/tests/test_solution.py @@ -8,6 +8,7 @@ import copy import os +import platform from pathlib import Path import numpy as np @@ -366,6 +367,8 @@ def test_components_by_element(s1, s2): "Na(1.0)": ["Na[+1]"], "Cl(-1.0)": ["Cl[-1]"], } + if platform.machine() == "arm64" and platform.system() == "Darwin": + pytest.skip(reason="arm64 not supported") s2.equilibrate() assert s2.get_components_by_element() == { "H(1.0)": ["H2O(aq)", "OH[-1]", "H[+1]", "HCl(aq)", "NaOH(aq)", "HClO(aq)", "HClO2(aq)"], @@ -401,6 +404,7 @@ def test_get_total_amount(s2): assert np.isclose(sox.get_total_amount("Fe", "mol").magnitude, 0.05) +@pytest.mark.skipif(platform.machine() == "arm64" and platform.system() == "Darwin", reason="arm64 not supported") def test_equilibrate(s1, s2, s5_pH): assert "H2(aq)" not in s1.components orig_pH = s1.pH @@ -505,6 +509,7 @@ def test_conductivity(s1, s2): assert np.isclose(s_kcl.conductivity.magnitude, 10.862, atol=0.45) +@pytest.mark.skipif(platform.machine() == "arm64" and platform.system() == "Darwin", reason="arm64 not supported") def test_arithmetic_and_copy(s2, s6): s6_scale = copy.deepcopy(s6) s6_scale *= 1.5 From e49a12fbb3e63ab4d47656a0c716977e0e5dc222 Mon Sep 17 00:00:00 2001 From: Ryan Date: Tue, 5 Mar 2024 16:13:31 -0500 Subject: [PATCH 2/6] CI: separate more comprehensive testing for release --- .github/workflows/post-process.yml | 65 ++++++++++++++++++++++++++---- 1 file changed, 58 insertions(+), 7 deletions(-) diff --git a/.github/workflows/post-process.yml b/.github/workflows/post-process.yml index c755ddd9..bbf36816 100644 --- a/.github/workflows/post-process.yml +++ b/.github/workflows/post-process.yml @@ -1,19 +1,70 @@ -name: Post-process +name: Post-merge +# triggered when PRs are merged into main branch on: - workflow_run: + pull_request: branches: - main types: - - completed - workflows: - # List all required workflow names here. - - 'testing' + - closed jobs: -# https://github.com/marketplace/actions/tag-release-on-push-action + lint: + if: github.event.pull_request.merged == true + runs-on: ubuntu-latest + strategy: + max-parallel: 1 + steps: + - uses: actions/checkout@v4 + with: + fetch-depth: 0 + - name: Set up Python + uses: actions/setup-python@v5 + with: + python-version: 3.11 + cache: pip + - name: Run pre-commit + run: | + pip install pre-commit + pre-commit run + + test-comprehensive: + needs: lint + strategy: + max-parallel: 6 + matrix: + python-version: ["3.9", "3.10", "3.11", "3.12"] + os: + - ubuntu-latest + - macos-latest + - windows-latest + - macos-14 + exclude: + - os: macos-14 + python-version: "3.9" + runs-on: ${{ matrix.os }} + steps: + - uses: actions/checkout@v4 + - name: Setup Python + uses: actions/setup-python@v5 + with: + python-version: ${{ matrix.python-version }}${{ matrix.dev }} + - name: Install test requirements + run: | + python -m pip install --upgrade pip + pip install -e ".[testing]" + - name: Run tests + run: | + pytest --cov=src/pyEQL --cov-report=xml + - uses: codecov/codecov-action@v4 + with: + token: ${{ secrets.CODECOV_TOKEN }} + file: ./coverage.xml + + # https://github.com/marketplace/actions/tag-release-on-push-action auto-gen-release: name: Tag/Release on merge of labeled PRs + needs: test-comprehensive runs-on: ubuntu-latest env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} From e262092bdbb771f42252090f68fbe6982dc99712 Mon Sep 17 00:00:00 2001 From: Ryan Date: Tue, 5 Mar 2024 17:21:29 -0500 Subject: [PATCH 3/6] Docs: add warning about Apple M1 support --- docs/engines.md | 10 ++++++++++ docs/installation.md | 13 ++++++++++--- 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/docs/engines.md b/docs/engines.md index b713b3dd..f3b9d131 100644 --- a/docs/engines.md +++ b/docs/engines.md @@ -21,6 +21,16 @@ as needed to particular use cases. `pyEQL` currently supports three modeling engines: `ideal`, `native`, and `phreeqc`, which are selected via the `engine` kwarg to `Solution.__init__()`. Each engine is briefly described below. +```{warning} +If you are using a Mac with an Apple M1, M2, etc. chip (i.e., Arm64 architecture), some features of `pyEQL` will be +unavailable. Specifically, anything which depends on PHREEQC (e.g., the +`equilibrate` method in the native engine and the entire + [`phreeqc` engine](engines.md#the-phreeqc-engine)) will not work. This is because `phreeqpython` is currently + not available for this platform. All other functions of `pyEQL` should work as expected. + +Feel free to post your experiences or proposed solutions at https://github.com/KingsburyLab/pyEQL/issues/109 +``` + ## The `'native'` engine (Default) The `native` engine is the default choice and was the only option available prior to diff --git a/docs/installation.md b/docs/installation.md index d206f613..b38d7ec9 100644 --- a/docs/installation.md +++ b/docs/installation.md @@ -33,22 +33,29 @@ You can tell if this is the case by typing the following command: ``` +``` $ python --version Python 2.7.12 - ``` This means Python 2.x is installed. If you run 'pip install' it will point to the Python 2.7 installation, but pyEQL only works on Python 3. So, try this: ``` - $ python3 --version Python 3.9.7 - ``` To get to Python 3.x, you have to type 'python3'. In this case, you would run 'pip3 install' + +```{warning} +If you are using a Mac with an Apple M1, M2, etc. chip (i.e., Arm64 architecture), some features of `pyEQL` will be +unavailable. Specifically, anything which depends on PHREEQC (e.g., the +`equilibrate` method in the native engine and the entire + [`phreeqc` engine](engines.md)) will not work. This is because `phreeqpython` is currently + not available for this platform. All other functions of `pyEQL` should work as expected. + +Feel free to post your experiences or proposed solutions at https://github.com/KingsburyLab/pyEQL/issues/109 ``` ## Other dependencies From 43bd634219a35a8326dc3076e66386d7629ac9d6 Mon Sep 17 00:00:00 2001 From: Ryan Date: Tue, 5 Mar 2024 14:03:42 -0500 Subject: [PATCH 4/6] utils: FormulaDict fix for python 3.12 --- src/pyEQL/utils.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/pyEQL/utils.py b/src/pyEQL/utils.py index 3adc8d5c..54bb5f57 100644 --- a/src/pyEQL/utils.py +++ b/src/pyEQL/utils.py @@ -99,5 +99,10 @@ def __setitem__(self, key, value): # sort contents anytime an item is set self.data = dict(sorted(self.items(), key=lambda x: x[1], reverse=True)) + # Necessary to define this so that .get() works properly in python 3.12+ + # see https://github.com/python/cpython/issues/105524 + def __contains__(self, key): + return standardize_formula(key) in self.data + def __delitem__(self, key): super().__delitem__(standardize_formula(key)) From 0012e68e6a3fa99034aa486fb274304be1ddf045 Mon Sep 17 00:00:00 2001 From: Ryan Date: Tue, 5 Mar 2024 14:36:41 -0500 Subject: [PATCH 5/6] NativeEOS: try/except for OSError (Apple M1) --- CHANGELOG.md | 3 +++ src/pyEQL/engines.py | 10 +++++++++- 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7c40ce34..d44f2cc7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added +- `NativeEOS` / `PhreeqcEOS`: Added `try`/`catch` so that `pyEQL` can still be used on platforms that PHREEQC does + not support, such as Apple Silicon. In such cases, functions like `equilibrate` that depend on PHREEQC will + raise errors, but everything else can still be used. - CI: Added Apple M1 runner (GitHub: `macos-14`) to the CI tests. ### Fixed diff --git a/src/pyEQL/engines.py b/src/pyEQL/engines.py index acb59182..bb645f58 100644 --- a/src/pyEQL/engines.py +++ b/src/pyEQL/engines.py @@ -159,7 +159,15 @@ def __init__( Path(os.path.dirname(__file__)) / "database" if self.phreeqc_db in ["llnl.dat", "geothermal.dat"] else None ) # create the PhreeqcPython instance - self.pp = PhreeqPython(database=self.phreeqc_db, database_directory=self.db_path) + # try/except added to catch unsupported architectures, such as Apple Silicon + try: + self.pp = PhreeqPython(database=self.phreeqc_db, database_directory=self.db_path) + except OSError: + logger.error( + "OSError encountered when trying to instantiate phreeqpython. Most likely this means you" + " are running on an architecture that is not supported by PHREEQC, such as Apple M1/M2 chips." + " pyEQL will work, but equilibrate() will have no effect." + ) # attributes to hold the PhreeqPython solution. self.ppsol = None # store the solution composition to see whether we need to re-instantiate the solution From b8b0705ddda00edbd74c27a359b7801b143ab743 Mon Sep 17 00:00:00 2001 From: Ryan Date: Tue, 5 Mar 2024 17:48:05 -0500 Subject: [PATCH 6/6] update CHANGELOG --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d44f2cc7..03a97095 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,7 +5,7 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). -## Unreleased +## [0.14.0] - 2024-03-05 ### Added