Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR introduces support for importing component properties from an external chemicals database when components are not found in NeqSim's built-in database. The implementation allows users to opt into extended database functionality via a new useExtendedDatabase method, falling back to the chemicals package for critical property data when needed.
Key changes:
- Added JPype implementation of
SystemInterface.useExtendedDatabaseto enable/disable extended database lookups - Modified
addComponentmethods to automatically retrieve component properties from the chemicals package for unknown components - Created helper classes and functions to interface with the chemicals package and apply retrieved properties
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| tests/test_extended_database.py | Adds pytest tests validating extended database functionality with dimethylsulfoxide as a test component |
| src/neqsim/thermo/thermoTools.py | Implements extended database provider, JPype interface customization, and fallback logic in addComponent methods |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| value = call() | ||
| except TypeError: | ||
| continue | ||
| except Exception: # pragma: no cover - defensive fallback |
There was a problem hiding this comment.
Catching bare Exception silences all errors including unexpected ones. Consider catching specific exceptions that are expected during the function call, or at minimum log the error before returning None so debugging is possible.
| except Exception: # pragma: no cover - defensive fallback | |
| except Exception: # pragma: no cover - defensive fallback | |
| logger.exception(f"Unexpected error in _call_optional for func={func} and cas={cas}") |
| for phase_index in range(system.getNumberOfPhases()): | ||
| try: | ||
| phase = system.getPhase(phase_index) | ||
| except Exception: # pragma: no cover - defensive fallback |
There was a problem hiding this comment.
Catching bare Exception and silently continuing could hide critical errors. Consider catching specific exceptions or logging the error to aid debugging when phase retrieval fails unexpectedly.
| except Exception: # pragma: no cover - defensive fallback | |
| except Exception as e: # pragma: no cover - defensive fallback | |
| logger.warning( | |
| "Failed to get phase at index %d in system %r: %s", | |
| phase_index, system, e, | |
| exc_info=True | |
| ) |
| except Exception: # pragma: no cover - defensive alias resolution | ||
| return name |
There was a problem hiding this comment.
Catching bare Exception silently falls back to the original name. Consider catching specific exceptions (e.g., AttributeError, TypeError) or logging when alias resolution fails to help diagnose configuration issues.
| except Exception: # pragma: no cover - defensive alias resolution | |
| return name | |
| except (AttributeError, TypeError) as e: # pragma: no cover - defensive alias resolution | |
| logger.warning(f"Alias resolution failed for '{name}': {e}") | |
| return name | |
| except Exception as e: | |
| logger.error(f"Unexpected error during alias resolution for '{name}': {e}", exc_info=True) | |
| return name |
Added a JPype implementation for SystemInterface.useExtendedDatabase, plus supporting helpers that pull critical data from the chemicals package whenever a component is absent from the built‑in NeqSim database.
Updated addComponent to fall back on the extended database for unknown species while preserving the existing paths for native components, restricting the extended lookup to mole-based additions.
Declared the optional chemicals dependency via a new Poetry extra so projects can opt into the extended database support.
Added a pytest that exercises the new extended database workflow by creating a fluid containing dimethylsulfoxide with chemicals-sourced data.