Skip to content

Commit 55eb674

Browse files
authored
Merge pull request #2 from SECQUOIA/fix-test-codequal
Refactor codebase and enhance CI for quality checks
2 parents f12160e + f45164b commit 55eb674

27 files changed

Lines changed: 350 additions & 271 deletions

.github/README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ A single, comprehensive workflow that replaces multiple separate workflows, redu
3636

3737
## Triggers
3838

39-
- **Push**: main, master, develop, test-devel branches
39+
- **Push**: main, master, develop branches
4040
- **Pull Request**: main, master, develop branches
4141
- **Schedule**: Weekly on Sundays at 3 AM UTC
4242
- **Manual**: Via workflow_dispatch

.github/workflows/ci.yml

Lines changed: 23 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ name: PharmaPy CI/CD Pipeline
22

33
on:
44
push:
5-
branches: [ main, master, develop, test-devel ]
5+
branches: [ main, master, develop ]
66
pull_request:
77
branches: [ main, master, develop ]
88
schedule:
@@ -157,11 +157,11 @@ jobs:
157157
python -m pytest tests/integration/test_assimulo.py::TestAssimuloIntegration::test_assimulo_version_compatibility -v
158158
python -m pytest tests/integration/test_assimulo.py::TestAssimuloIntegration::test_sundials_integration -v
159159
160-
# Installation script testing - only on push to specific branches
160+
# Installation script testing - runs on all pushes
161161
test-installation-scripts:
162162
name: Installation Scripts
163163
timeout-minutes: 20 # Prevent long-running jobs
164-
if: github.event_name == 'push' && (contains(github.ref, 'main') || contains(github.ref, 'master') || contains(github.ref, 'test-devel'))
164+
if: github.event_name == 'push'
165165
runs-on: ${{ matrix.os }}
166166
strategy:
167167
fail-fast: false
@@ -195,10 +195,9 @@ jobs:
195195
conda activate pharmapy-ci-test
196196
python -c "import PharmaPy; print('Unix script installation successful')"
197197
198-
# Code quality and documentation - only on main branches
198+
# Code quality and documentation - run on every push
199199
quality-and-docs:
200200
name: Code Quality & Documentation
201-
if: github.ref == 'refs/heads/main' || github.ref == 'refs/heads/master'
202201
runs-on: ubuntu-latest
203202

204203
steps:
@@ -220,13 +219,31 @@ jobs:
220219
pip install -e .
221220
pip install -r requirements-dev.txt
222221
222+
- name: Install pandoc (for documentation)
223+
shell: bash -l {0}
224+
run: |
225+
conda install -c conda-forge pandoc --yes
226+
227+
- name: Install assimulo (for code quality checks)
228+
shell: bash -l {0}
229+
continue-on-error: true
230+
run: |
231+
conda install -c conda-forge assimulo --yes || pip install assimulo || echo "Assimulo installation failed, but continuing"
232+
223233
- name: Run linting
224234
shell: bash -l {0}
225235
continue-on-error: true
226236
run: |
227237
flake8 PharmaPy/ --count --select=E9,F63,F7,F82 --show-source --statistics
228238
flake8 PharmaPy/ --count --exit-zero --max-complexity=10 --max-line-length=127 --statistics
229239
240+
- name: Check code formatting with black
241+
shell: bash -l {0}
242+
continue-on-error: true
243+
run: |
244+
echo "Checking code formatting with black..."
245+
black --check --diff PharmaPy/ tests/ *.py --exclude="/(\.git|\.venv|\.tox|build|dist|\.eggs)/"
246+
230247
- name: Generate coverage report
231248
shell: bash -l {0}
232249
run: |
@@ -258,10 +275,9 @@ jobs:
258275
name: documentation
259276
path: doc/_build/html/
260277

261-
# Build and packaging - only on main branches and releases
278+
# Build and packaging - runs on all pushes
262279
build-and-package:
263280
name: Build & Package
264-
if: github.ref == 'refs/heads/main' || github.ref == 'refs/heads/master' || startsWith(github.ref, 'refs/tags/')
265281
runs-on: ubuntu-latest
266282

267283
steps:

PharmaPy/CheckModule.py

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88
import json
99
import pathlib
1010

11-
from PharmaPy.Errors import PharmaPySpecificationError
1211
import warnings
1312

1413

PharmaPy/Crystallizers.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2426,6 +2426,7 @@ def energy_balances(
24262426
vol,
24272427
mu_n,
24282428
h_in,
2429+
heat_prof=False,
24292430
):
24302431

24312432
rho_susp, rho_in = rhos

PharmaPy/Drying_Model.py

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,18 +9,13 @@
99
from assimulo.problem import Explicit_Problem
1010
from assimulo.solvers import CVode
1111
import matplotlib.pyplot as plt
12-
import scipy
1312
from scipy.interpolate import CubicSpline
1413

1514
from PharmaPy.Phases import classify_phases
16-
from PharmaPy.MixedPhases import Cake
1715
from PharmaPy.SolidLiquidSep import high_resolution_fvm, get_sat_inf, upwind_fvm
18-
from PharmaPy.NameAnalysis import get_dict_states
1916

2017
# from PharmaPy.Interpolation import SplineInterpolation
21-
from PharmaPy.general_interpolation import define_initial_state
2218
from PharmaPy.Commons import (
23-
reorder_pde_outputs,
2419
eval_state_events,
2520
handle_events,
2621
unpack_discretized,
@@ -483,7 +478,7 @@ def energy_balance(
483478

484479
dTcond_dt = (-drying_terms + heat_transf - heat_loss_cond) / denom_cond
485480

486-
## ---- Trial for lumping both cond/gas phase into one
481+
# ---- Trial for lumping both cond/gas phase into one
487482
# dTtotal_dt = (conv_term + sensible_heat - drying_terms - heat_loss_cond)/ (denom_gas + denom_cond)
488483

489484
# dTg_dt, dTcond_dt = dTtotal_dt, dTtotal_dt

PharmaPy/DynamicExtraction.py

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,17 @@
11
import numpy as np
2-
from scipy import linalg
32
from scipy.optimize import root
43
from assimulo.problem import Implicit_Problem
54

65
from PharmaPy.Phases import classify_phases
76
from PharmaPy.Connections import get_inputs_new
8-
from PharmaPy.Commons import unpack_discretized, retrieve_pde_result, flatten_states
7+
from PharmaPy.Commons import unpack_discretized, retrieve_pde_result
98
from PharmaPy.Results import DynamicResult
109
from PharmaPy.Streams import LiquidStream
1110

1211
from PharmaPy.Extractors import BatchExtractor
1312
from PharmaPy.Plotting import plot_distrib
1413

1514
from assimulo.solvers import IDA
16-
from assimulo.solvers import Radau5DAE
1715

1816
import copy
1917

PharmaPy/Evaporators.py

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,15 +13,14 @@
1313
unpack_states,
1414
flatten_states,
1515
)
16-
from PharmaPy.Connections import get_inputs, get_inputs_new
16+
from PharmaPy.Connections import get_inputs_new
1717
from PharmaPy.Streams import LiquidStream, VaporStream
1818
from PharmaPy.Phases import LiquidPhase, VaporPhase, classify_phases
1919

2020
from PharmaPy.Results import DynamicResult
2121
from PharmaPy.Plotting import plot_function
2222

2323
from scipy.optimize import fsolve
24-
import matplotlib.pyplot as plt
2524
from matplotlib.ticker import AutoMinorLocator
2625

2726
from assimulo.problem import Implicit_Problem
@@ -1230,7 +1229,7 @@ def plot_profiles(self, pick_comp=None, **fig_kwargs):
12301229
The default is None.
12311230
**fig_kwargs : keyword arguments
12321231
keyword arguments to be passed to the construction of fig and
1233-
axes object of matplotlib (plt.subplots(**kwargs)).
1232+
axes object of matplotlib (``plt.subplots(**kwargs)``).
12341233
Do not use nrows or ncols arguments, since the plot grid is already
12351234
defined by PharmaPy
12361235
@@ -2093,7 +2092,7 @@ def plot_profiles(self, pick_comp=None, vol_plot=False, **fig_kwargs):
20932092
mol_liq-mol_vap vs t is plotted.
20942093
**fig_kwargs : keyword arguments
20952094
keyword arguments to be passed to the construction of fig and
2096-
axes objects of matplotlib (plt.subplots(**kwargs)).
2095+
axes objects of matplotlib (``plt.subplots(**kwargs)``).
20972096
Do not use nrows or ncols arguments, since the plot grid is already
20982097
defined by PharmaPy
20992098

PharmaPy/Extractors.py

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@
1111
from PharmaPy.Commons import mid_fn
1212
from PharmaPy.Phases import LiquidPhase
1313
from PharmaPy.Streams import LiquidStream
14-
from PharmaPy.Connections import get_inputs_new
1514

1615
from PharmaPy.Results import DynamicResult
1716

PharmaPy/Interpolation.py

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88

99
import numpy as np
1010
import matplotlib.pyplot as plt
11-
from scipy.interpolate import CubicSpline
1211
from scipy.special import comb
1312

1413

PharmaPy/Kinetics.py

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -125,14 +125,17 @@ class RxnKinetics:
125125
r"""
126126
Create a reaction kinetics object. Reaction rate r\ :sub:`i` is assumed to
127127
have the following functional form:
128-
r\ :sub:`i` = f\ :sub:`1` (T) * f\ :sub:`2` ( C\ :sub:`1`, ..., C\ :sub:`n_comp`)
128+
129+
r\ :sub:`i` = f\ :sub:`1` (T) * f\ :sub:`2` ( C\ :sub:`1`, ..., C\ :sub:`n_comp`)
129130
130131
with the temperature-dependent term f\ :sub:`1` given by:
131-
f\ :sub:`1` = k\ :sub:`i` * exp(- Ea\ :sub:`i`/R/T)
132+
133+
f\ :sub:`1` = k\ :sub:`i` * exp(- Ea\ :sub:`i`/R/T)
132134
133135
Composition-dependent term f\ :sub:`2` can be passed as a user-defined
134136
function. If not given, f\ :sub:`2` is assumed to be of the form:
135-
f\ :sub:`2` = prod\ :sub:`j in reactants for rxn i` C\ :sub:`j` (alpha\ :sub:`{i,j}`)
137+
138+
f\ :sub:`2` = prod\ :sub:`j in reactants for rxn i` C\ :sub:`j` (alpha\ :sub:`{i,j}`)
136139
137140
where alpha\ :sub:`{i,j}` values are determined automatically by PharmaPy from
138141
the stoichiometric matrix of the reaction system. Custom reaction
@@ -167,6 +170,7 @@ class RxnKinetics:
167170
pure-component json file. If 'rxn_list' is None, then both
168171
stoichiometric_matrix' and 'partic_species' have to be passed
169172
(see below). The default is None.
173+
170174
stoiciometric_matrix : numpy array, optional
171175
stoichiometric matrix for the set of reactions. It must have
172176
n_rxn rows and n_comp columns, so the element (i, j) represents

0 commit comments

Comments
 (0)