Skip to content

Deprecate legacy convenience headers #1984

Merged
speth merged 5 commits intoCantera:mainfrom
ischoegl:deprecate-headers
Sep 23, 2025
Merged

Deprecate legacy convenience headers #1984
speth merged 5 commits intoCantera:mainfrom
ischoegl:deprecate-headers

Conversation

@ischoegl
Copy link
Copy Markdown
Member

@ischoegl ischoegl commented Sep 20, 2025 •
edited
Loading

Changes proposed in this pull request

The convenience headers thermo.h, kinetics.h and transport.h have been marked as 'superseded by core.h' since Cantera 3.0. This PR deprecates them with Cantera 3.2.

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

@ischoegl ischoegl force-pushed the deprecate-headers branch 3 times, most recently from ec30f6a to 6ea7e11 Compare September 20, 2025 17:37
@ischoegl ischoegl marked this pull request as ready for review September 20, 2025 18:38
@ischoegl ischoegl requested a review from a team September 20, 2025 21:39
speth
speth requested changes Sep 22, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@speth speth left a comment

Choose a reason for hiding this comment

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

If core.h is meant to be a true replacement for these other convenience headers, then I think it should also include a few other headers as well:

  • ThermoFactory.h
  • KineticsFactory.h
  • TransportFactory.h
  • Species.h
  • Reaction.h

The less users need to think about the factory classes (as opposed to the newX functions) the better.

@ischoegl
Copy link
Copy Markdown
Member Author

ischoegl commented Sep 22, 2025 •
edited
Loading

If core.h is meant to be a true replacement for these other convenience headers, then I think it should also include a few other headers as well:

  • ThermoFactory.h
  • KineticsFactory.h
  • TransportFactory.h
  • Species.h
  • Reaction.h

The less users need to think about the factory classes (as opposed to the newX functions) the better.

Unfortunately, I disagree with the factory headers - I truly believe that any new code should go through newSolution/newInterface. All instances where the factories are called directly (which I had to touch in this PR) are legacy code, and I am not in favor of continuing this practice. I can see the point of Species.h and Reaction.h, as well as modifying the 'replacement options' in the pragma warnings to also point to factories. If a user really insists on side-lining Solution, they should not be able to rely on a convenience header.

@ischoegl ischoegl requested a review from speth September 22, 2025 16:57
speth
speth approved these changes Sep 23, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@speth speth left a comment

Choose a reason for hiding this comment

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

OK, with the updated deprecation message, I think this is fine.

@speth speth merged commit 99550d0 into Cantera:main Sep 23, 2025
46 of 49 checks passed
@ischoegl ischoegl deleted the deprecate-headers branch September 23, 2025 01:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@speth speth speth approved these changes

Assignees

No one assigned

Labels

CI compiling

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@ischoegl @speth