Skip to content

Make sourcegen more pythonic#1893

Merged
ischoegl merged 11 commits intoCantera:mainfrom
ischoegl:pythonic-sourcegen
May 26, 2025
Merged

Make sourcegen more pythonic#1893
ischoegl merged 11 commits intoCantera:mainfrom
ischoegl:pythonic-sourcegen

Conversation

@ischoegl
Copy link
Copy Markdown
Member

@ischoegl ischoegl commented May 25, 2025 •
edited
Loading

Changes proposed in this pull request

This PR is mainly housekeeping, with minimal actual code changes:

  • Adopt standard Python project structure for interfaces/sourcegen with lowercase file names
  • Use pyproject.toml to make sourcegen installable via pip install interfaces/sourcegen
  • Make sourcegen a CLI utility

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

Addresses Cantera/enhancements#220

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

This is now possible:

% pip install interfaces/sourcegen
% sourcegen --help
% sourcegen --api=clib --output=.

wIth pip install -e interfaces/sourcegen used while working on sourcegen.

The tree structure (ignoring YAML and template files) is now

% tree -I '__pycache__|*.pyc|.git|*.yaml|*.in|*egg-info|build' --filesfirst
.
├── pyproject.toml
└── src
    └── sourcegen
        ├── __init__.py
        ├── _helpers.py
        ├── api.py
        ├── dataclasses.py
        ├── generator.py
        ├── clib
        │   ├── __init__.py
        │   └── generator.py
        ├── csharp
        │   ├── __init__.py
        │   └── generator.py
        ├── headers
        │   ├── __init__.py
        │   ├── generator.py
        │   ├── headerfiles.py
        │   └── tagfiles.py
        └── yaml
            ├── __init__.py
            └── generator.py

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 May 25, 2025
@ischoegl ischoegl mentioned this pull request May 25, 2025
Closed
9 tasks
@ischoegl ischoegl force-pushed the pythonic-sourcegen branch 3 times, most recently from c29a03b to 36bea6d Compare May 25, 2025 15:45
@ischoegl ischoegl marked this pull request as ready for review May 25, 2025 16:19
@ischoegl ischoegl requested a review from a team May 25, 2025 16:19
@ischoegl
Copy link
Copy Markdown
Member Author

ischoegl commented May 25, 2025

Ready for a review. @bryanwweber ... based on your Python background, could you have a look at this?

@bryanwweber
Copy link
Copy Markdown
Member

bryanwweber commented May 25, 2025 •
edited
Loading

I think we have 2 options to proceed:

  1. Use sourcegen/src/sourcegen (as in this PR) and specify the module should be installed with pip or other tools into the venv with SCons and other dependencies
  2. Retain the sourcegen/sourcen structure, add a __main__.py file that does the actual running of the package, and use python -m sourcegen.sourcegen (I think, this might need a little experimentation) [Edit]: Actually, I think in this case we'd want interfaces/sourcegen/ to be the root of the package and it'd be python -m interfaces/sourcegen. Maybe.

In the first option, we kind of end up polluting the venv, plus local changes made to sourcegen won't get picked up unless we use pip install -e. In the second option, the code is always loaded from the actual source (not the installed source) so it's always up-to-date and there's no "pollution" in the venv.

What do you think @ischoegl? I think either way is fine, but I have different suggestions for the actual code depending which route we take 😄

@ischoegl
Copy link
Copy Markdown
Member Author

ischoegl commented May 25, 2025 •
edited
Loading

@bryanwweber … given that this utility is meant to be run within a venv (presumably it will only be used by developers), I don’t believe that pollution is an issue. Gaining the convenience of a CLI (rather than the longish python -m interfaces/sourcegen) makes the pip route significantly more convenient: especially when testing! I can update the docs to point out the -e option (as well as that things should be run within a venv).

bryanwweber
bryanwweber reviewed May 26, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@bryanwweber bryanwweber left a comment

Choose a reason for hiding this comment

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

Aside from the minor updates here, I'd suggest adding a __main__.py file that calls api.main() in an if __name__ block. That way, the package can also be executed as python -m sourcegen which may be more convenient in some cases, similar to python -m pip.

ischoegl added 2 commits May 26, 2025 10:33
@ischoegl
[sourcegen] Make sourcegen installation flexible
e0605cd 
Ensure that all folders are found independent of the sourcegen
installation location
@ischoegl
[docs] Update sourcegen documentation
307c623
@ischoegl ischoegl force-pushed the pythonic-sourcegen branch from 36bea6d to 85a87b1 Compare May 26, 2025 15:41
@ischoegl
Copy link
Copy Markdown
Member Author

ischoegl commented May 26, 2025

Thanks, @bryanwweber ... all done! I had never tried hatchling; it looks nice!

@ischoegl ischoegl requested a review from a team May 26, 2025 16:15
bryanwweber
bryanwweber reviewed May 26, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@bryanwweber bryanwweber left a comment

Choose a reason for hiding this comment

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

One more small suggestion, sorry I didn't catch this the first time

@ischoegl ischoegl force-pushed the pythonic-sourcegen branch from 85a87b1 to ae174d4 Compare May 26, 2025 17:07
@ischoegl
Copy link
Copy Markdown
Member Author

ischoegl commented May 26, 2025

@bryanwweber ... done!

@ischoegl ischoegl requested a review from a team May 26, 2025 17:38
bryanwweber
bryanwweber approved these changes May 26, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@bryanwweber bryanwweber 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 !

@ischoegl ischoegl merged commit e87aa06 into Cantera:main May 26, 2025
47 of 49 checks passed
@ischoegl ischoegl deleted the pythonic-sourcegen branch May 26, 2025 21:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bryanwweber bryanwweber bryanwweber approved these changes

Assignees

No one assigned

Labels

clib

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@ischoegl @bryanwweber