Skip to content

Conversation

@ischoegl
Copy link
Member

@ischoegl ischoegl commented Oct 15, 2025

Changes proposed in this pull request

  • Address issue where \param[out] of type vector<> is not scaffolded correctly (was not used by CLib, but was also not covered by exception)
  • Fix missing int crosswalk for methods that do not take a parameter
  • Introduce Jinja2 macros to improve template readability
  • Extract \deprecated message from C++ documentation and propagate annotation to generated CLib
  • Remove _auto from the header configuration files. It is not needed.
  • Document inclusion of custom code
  • Address minor issues in docstrings and documentation
  • Cherry-pick commits necessary to fix incompatibilities with MATLAB as uncovered in [MATLAB] Migrate to generated Clib #1997

If applicable, fill in the issue number this pull request is fixing

Closes #2000

Closes Cantera/enhancements#232

Partially implements Cantera/enhancements#230

If applicable, provide an example illustrating new features this pull request is introducing

The issue pointed out in #1995 (comment) is resolved as:

    int32_t domain_getValues(int32_t handle, const char* component, int32_t valuesLen, double* values)
    {
        // method: virtual void Domain1D::getValues(const string&, vector<double>&)
        try {
            vector<double> values_(valuesLen);
            Domain1DCabinet::at(handle)->getValues(component, values_);
            std::copy(values_.begin(), values_.end(), values);
            return 0;
        } catch (...) {
            return handleAllExceptions(-1, ERR);
        }
    }

Deprecation warnings are now propagated

    /**
     *  Add a ReactorSurface object to a Reactor object.
     *
     *  Wraps C++ setter: `void ReactorBase::addSurface(shared_ptr<ReactorBase>)`
     *
     *  @param handle       Handle to queried ReactorBase object.
     *  @param surf         Integer handle to ReactorBase object. Undocumented.
     *
     *  @deprecated Per C++ annotation: Unused. To be removed after Cantera 3.2.
     */
    int32_t reactor_addSurface(int32_t handle, int32_t surf);

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 added the clib label Oct 15, 2025
@ischoegl ischoegl marked this pull request as ready for review October 15, 2025 13:58
@ischoegl ischoegl requested a review from a team October 15, 2025 13:58
@codecov
Copy link

codecov bot commented Oct 15, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 75.23%. Comparing base (a8ddb8d) to head (df97241).
⚠️ Report is 16 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2011   +/-   ##
=======================================
  Coverage   75.23%   75.23%           
=======================================
  Files         454      454           
  Lines       56756    56756           
  Branches     9373     9373           
=======================================
  Hits        42703    42703           
  Misses      10874    10874           
  Partials     3179     3179           

☔ 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.

@ischoegl ischoegl force-pushed the sourcegen-glitches branch 2 times, most recently from 0561835 to c63f206 Compare October 16, 2025 12:27
@ischoegl ischoegl changed the title Address sourcegen glitches sourcegen improvements Oct 17, 2025
@ischoegl ischoegl marked this pull request as draft October 17, 2025 22:23
@ischoegl ischoegl removed the request for review from a team October 17, 2025 22:23
@ischoegl ischoegl marked this pull request as ready for review October 18, 2025 01:44
@ischoegl ischoegl requested a review from a team October 18, 2025 01:57
@ischoegl ischoegl force-pushed the sourcegen-glitches branch 2 times, most recently from 544f0c1 to 3435360 Compare October 18, 2025 13:49
While '_auto' was useful for testing purposes, it is no longer needed.
All relevant files start with 'ct', which is sufficient.
Docstrings containing '%Cantera' cause crashes in the MATLAB build
process, as '%' is reserved for comments.
@ischoegl
Copy link
Member Author

ischoegl commented Oct 18, 2025

@speth ... I believe this is ready for a review. The only question I have is whether it's time to remove the disclaimers before releasing 3.2.

 *  @warning  This library is an experimental part of the %Cantera API and
 *      may be changed without notice.

My main reservations are related to Cantera/enhancements#228, but I don't know when/whether this would be implemented. Otherwise, this is pretty much converged.

PS: one other thing that remains to be done is to issue compile time deprecation warnings for the legacy CLib.

@speth
Copy link
Member

speth commented Oct 19, 2025

The only question I have is whether it's time to remove the disclaimers before releasing 3.2.

 *  @warning  This library is an experimental part of the %Cantera API and
 *      may be changed without notice.

I think it's worth reserving the option of making breaking API changes to this interface for at least one release.

Copy link
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.

Thanks, @ischoegl. This all looks good to me.

@ischoegl ischoegl merged commit ab988c2 into Cantera:main Oct 19, 2025
48 of 49 checks passed
@ischoegl ischoegl deleted the sourcegen-glitches branch October 19, 2025 04:52
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.

Sourcegen does not differentiate input/output for vector<> arguments Leverage Jinja Capabilities in sourcegen

2 participants