Skip to content

Deprecate ReactorBase::addThermo#1966

Merged
ischoegl merged 1 commit intoCantera:mainfrom
speth:deprecate-setThermo
Sep 7, 2025
Merged

Deprecate ReactorBase::addThermo#1966
ischoegl merged 1 commit intoCantera:mainfrom
speth:deprecate-setThermo

Conversation

@speth
Copy link
Copy Markdown
Member

@speth speth commented Sep 7, 2025

The virtual ReactorBase::addThermo does not function as intended, since overrides are not executed when this method is called from the base class constructor. Derived classes needing customized behavior must implement it in their own constructors or in overrides of the initialize method.

This erroneous usage was mostly invisible because we have only been using these overrides to ensure that the ThermoPhase objects used in the "ideal gas" reactor types are in fact ideal gasses.

Changes proposed in this pull request

  • Move checks out of overrides of ReactorBase::addThermo
  • Deprecate ReactorBase::addThermo

Checklist

  • The pull request includes a clear description of this code change
  • Commit messages have short titles and reference relevant issues
  • Build passes (scons build & scons test) and unit tests address code coverage
  • Style & formatting of contributed code follows contributing guidelines
  • The pull request is ready for review

This method does not function as intended, since overrides are not executed
when this method is called from the base class constructor. Derived classes
needing customized behavior must implement it in their own constructors or
in overrides of the initialize method.
@speth speth added the Reactors label Sep 7, 2025
@codecov
Copy link
Copy Markdown

codecov bot commented Sep 7, 2025

Codecov Report

❌ Patch coverage is 41.17647% with 10 lines in your changes missing coverage. Please review.
✅ Project coverage is 75.04%. Comparing base (71ffab4) to head (1f7c11c).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
src/zeroD/IdealGasConstPressureMoleReactor.cpp 0.00% 1 Missing and 1 partial ⚠️
src/zeroD/IdealGasConstPressureReactor.cpp 33.33% 1 Missing and 1 partial ⚠️
src/zeroD/IdealGasMoleReactor.cpp 0.00% 1 Missing and 1 partial ⚠️
src/zeroD/IdealGasReactor.cpp 0.00% 1 Missing and 1 partial ⚠️
src/zeroD/ReactorBase.cpp 75.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1966      +/-   ##
==========================================
- Coverage   75.04%   75.04%   -0.01%     
==========================================
  Files         450      450              
  Lines       56237    56236       -1     
  Branches     9302     9302              
==========================================
- Hits        42203    42201       -2     
+ Misses      10901    10898       -3     
- Partials     3133     3137       +4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Copy Markdown
Member

@ischoegl ischoegl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for catching this, @speth!

@ischoegl ischoegl merged commit 4cac7ac into Cantera:main Sep 7, 2025
48 of 51 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants