-
-
Notifications
You must be signed in to change notification settings - Fork 356
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Experimental clib
from doxygen
#1835
Conversation
f37d89b
to
6226766
Compare
6226766
to
4f4d79f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks Ingmar! I know this is still in draft but I wanted to leave a small comment that might be intrusive later
ee04eb8
to
22be9f9
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1835 +/- ##
=======================================
Coverage 74.39% 74.39%
=======================================
Files 382 382
Lines 53330 53330
Branches 9022 9021 -1
=======================================
Hits 39676 39676
Misses 10607 10607
Partials 3047 3047 ☔ View full report in Codecov by Sentry. |
22be9f9
to
a06ec48
Compare
@speth and @bryanwweber ... this is ready for a review.
|
a06ec48
to
97bfa50
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for all this work Ingmar! Nearly all of my comments here are suggestions to improve the type hinting. In some cases, you'll need to add an import (I think just for Iterable
). Obviously these don't affect the runtime, but my thought was that if we're going to have type hints they should at least be complete 😄
97bfa50
to
2350a3d
Compare
Thanks, @bryanwweber, for the prompt review! I agree that having done most of the type hints already, it makes sense to take it all the way. I adopted almost all of the suggestions directly and added some conversions, so the |
739aa5d
to
d48f3e8
Compare
This comment was marked as outdated.
This comment was marked as outdated.
Also fix issues caused by differences of doxygen and Cantera whitespace conventions in argument lists and add recipes for functions used in googletests.
19ca9a4
to
3dc73bd
Compare
Yes, I believe this can be leveraged in the near future (especially for MATLAB, as the hand-coded part covers only a portion of the actual API). Fortran could be done in analogy to CLib (without depending on it).
I had already created a tracking issue for this: Cantera/enhancements#220. For now, the "3"s are needed; a future removal is imho trivial. I can likely handle the transition, but am not planning on investing in code generation for other APIs. |
Adding tests to clang-compiler workflow.
3dc73bd
to
83442a2
Compare
@speth ... please let me know in case there's anything else. At this point, I have taken care of all review comments. |
Changes proposed in this pull request
This PR seeks to implement a basis for future auto-generated Cantera APIs that are fully documented based on information provided in C++ docstrings.
clib
version ("clib-experimental
")clib
scaffolder is based on doxygen tag files and associated XML anchorfilesinterfaces/sourcegen/sourcegen/_data/README.md
)clib
googletests is adopted for illustration and code coverage astest-clib-experimental
clang-compiler
actionThis proof of concept re-implements approaches of the traditional CLib library and is newr feature-complete, although the conversion of
zeroD
andoneD
objects is currently omitted.One other currently omitted feature involves the inclusion of code that is difficult to automate, although it is expected to be rare;in many instances, tweaks to the C++ interface may be the go-to approach instead. Edit: custom code is now supported.The configuration involves YAML markup that only uses minimal information for each entry, e.g.
which produces fully documented declarations
and implementations (click to expand)
Here is an example for a full YAML input used for code generation
sol3_auto.yaml (click to expand)
If applicable, fill in the issue number this pull request is fixing
See Cantera/enhancements#39
Partially addresses Cantera/enhancements#220
If applicable, provide an example illustrating new features this pull request is introducing
Run the following command from the Cantera root folder:
A partial test suite based on the traditional CLib ensures that code performs as expected:
Checklist
scons build
&scons test
) and unit tests address code coverage