Skip to content

Commit 3c8a2e7

Browse files
committed
Fix code review issues: NULL check, CMake syntax, and redundant config
- Add missing NULL check after PyDict_New() in run_simulation_with_params - Fix CMake if() syntax: remove dollar-brace from variable references - Remove redundant CFD_ROOT fallback in Windows cibuildwheel config - Add dynamic version from setuptools-scm with fallbacks - Add comment explaining Py_LIMITED_API build system requirement
1 parent f9d215b commit 3c8a2e7

4 files changed

Lines changed: 31 additions & 6 deletions

File tree

CMakeLists.txt

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ option(CFD_USE_STABLE_ABI "Use Python stable ABI (abi3)" OFF)
1212
find_package(Python 3.8 REQUIRED COMPONENTS Interpreter Development.Module)
1313

1414
# Enable stable ABI (abi3) for Python 3.8+ if requested
15-
if(CFD_USE_STABLE_ABI AND ${Python_VERSION} VERSION_GREATER_EQUAL "3.8")
15+
if(CFD_USE_STABLE_ABI AND Python_VERSION VERSION_GREATER_EQUAL "3.8")
1616
set(CMAKE_C_VISIBILITY_PRESET hidden)
1717
set(CMAKE_CXX_VISIBILITY_PRESET hidden)
1818
add_definitions(-DPy_LIMITED_API=0x03080000)
@@ -84,7 +84,7 @@ set_target_properties(cfd_python PROPERTIES
8484
)
8585

8686
# Use stable ABI if requested and available
87-
if(CFD_USE_STABLE_ABI AND ${Python_VERSION} VERSION_GREATER_EQUAL "3.8")
87+
if(CFD_USE_STABLE_ABI AND Python_VERSION VERSION_GREATER_EQUAL "3.8")
8888
target_compile_definitions(cfd_python PRIVATE Py_LIMITED_API=0x03080000)
8989
endif()
9090

cfd_python/__init__.py

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,17 @@
2121
- OUTPUT_CSV_STATISTICS: Global statistics (CSV)
2222
"""
2323

24-
__version__ = "0.3.0"
24+
# Get version from package metadata (setuptools-scm) or fall back to C module
25+
try:
26+
from importlib.metadata import version, PackageNotFoundError
27+
try:
28+
__version__ = version("cfd-python")
29+
except PackageNotFoundError:
30+
# Package not installed, try C module version
31+
__version__ = None
32+
except ImportError:
33+
# Python < 3.8 fallback
34+
__version__ = None
2535

2636
# Core exports that are always available
2737
_CORE_EXPORTS = [
@@ -76,6 +86,10 @@
7686
# Import the C extension module to access dynamic solver constants
7787
from . import cfd_python as _cfd_module
7888

89+
# Fall back to C module version if metadata lookup failed
90+
if __version__ is None:
91+
__version__ = getattr(_cfd_module, '__version__', '0.0.0')
92+
7993
# Dynamically export all SOLVER_* constants from the C module
8094
# This allows new solvers to be automatically available without
8195
# updating this file
@@ -90,4 +104,6 @@
90104

91105
except ImportError:
92106
# Development mode - module not yet built
93-
__all__ = _CORE_EXPORTS
107+
__all__ = _CORE_EXPORTS
108+
if __version__ is None:
109+
__version__ = "0.0.0-dev"

pyproject.toml

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -104,9 +104,8 @@ environment = { CMAKE_BUILD_TYPE = "Release", CFD_STATIC_LINK = "ON", CFD_USE_ST
104104
[tool.cibuildwheel.windows]
105105
# Windows-specific configuration
106106
# Build CFD library first (static), then build Python extension
107-
# CFD_ROOT environment variable specifies the CFD library location (default: ../cfd)
107+
# CFD_ROOT is set in environment below
108108
before-build = [
109-
"if not defined CFD_ROOT set CFD_ROOT=../cfd",
110109
"cd %CFD_ROOT% && cmake -B build -DCMAKE_BUILD_TYPE=Release -DBUILD_SHARED_LIBS=OFF",
111110
"cd %CFD_ROOT% && cmake --build build --config Release",
112111
]

src/cfd_python.c

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,9 @@
1+
// Py_LIMITED_API should be defined via build system (CMake) before including Python.h
2+
// for stable ABI support. The CMake option CFD_USE_STABLE_ABI controls this.
3+
// When enabled, it targets Python 3.8+ stable ABI (0x03080000) for binary compatibility
4+
// across Python versions. Do NOT define it here unconditionally as it requires linking
5+
// against python3.lib which may not be available in all environments.
6+
17
#define PY_SSIZE_T_CLEAN
28
#include <Python.h>
39
#include <stdio.h>
@@ -375,6 +381,10 @@ static PyObject* run_simulation_with_params(PyObject* self, PyObject* args, PyOb
375381

376382
// Create results dictionary
377383
PyObject* results = PyDict_New();
384+
if (results == NULL) {
385+
free_simulation(sim_data);
386+
return NULL;
387+
}
378388

379389
// Get velocity magnitude
380390
FlowField* field = sim_data->field;

0 commit comments

Comments
 (0)