Consolidate type hinting in cti2yaml

[ischoegl]

Changes proposed in this pull request

This PR introduces a consolidation of type hinting - having stub files separate from pure Python files is not ideal. The consolidation triggers more thorough type checking.

Aside: this could be further updated by forcing PascalCase class names; at the same time, this would expand the scope significantly.

AI Statement (required)

• Extensive use of generative AI. Significant portions of code or documentation were generated with AI, including logic and implementation decisions. All generated code and documentation were reviewed and understood by the contributor.

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
• AI Statement is included
• The pull request is ready for review

[codecov]

Codecov Report

❌ Patch coverage is 88.99371% with 35 lines in your changes missing coverage. Please review.
✅ Project coverage is 76.80%. Comparing base (ecced37) to head (9b22835).

Files with missing lines	Patch %	Lines
interfaces/cython/cantera/cti2yaml.py	88.99%	24 Missing and 11 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2057      +/-   ##
==========================================
+ Coverage   76.75%   76.80%   +0.04%     
==========================================
  Files         457      457              
  Lines       53699    53855     +156     
  Branches     9121     9127       +6     
==========================================
+ Hits        41219    41362     +143     
- Misses       9375     9382       +7     
- Partials     3105     3111       +6     

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

[bryanwweber]

I know this is still in draft but I wanted to leave a couple comments while you're working on it

[bryanwweber]

Types for literals like this will be inferred by the type checker, so you can remove this

[ischoegl]

Agreed, but the mypy settings are quite aggressive so this errors if left off.

[bryanwweber]

Then I think we should mark them as Final[float] etc. so that we're actually adding information here.

[ischoegl]

Removed the type as it appears to work after all.

Nevermind: worked on my machine, but not on CI. The --verifytypes option makes the Final[float] etc. necessary.

[TimothyEDawson]

I would have gladly consolidated the remaining type stubs for the various conversion scripts, but was told that they're not a high priority. As you can see, it can get pretty annoying! I'll leave a few comments based on the errors I'm seeing right now.

Note that this command should report no errors:

mypy --config-file interfaces/cython/pyproject.toml interfaces/cython/cantera

[TimothyEDawson]

This is flagged as an error: unused-ignore within the mypy check, but I can see how it might be required for the stubtest checks. I'm not sure what to do about that, exactly, but we might need to change unused-ignore to a warning instead of an error. That should be an option that can be set in the pyproject.toml file.

[ischoegl]

Added disable_error_code = ["unused-ignore"] to pyproject.toml

[ischoegl]

I’m going to leave this PR here (in draft mode). Full disclosure: I was testing automated coding for this merge of stub files and Python files. I’m pleased with the outcome, but don’t want to work on this while I cannot get mypy etc. to run locally. Will wait for the docs.

[TimothyEDawson]

I’m going to leave this PR here (in draft mode). Full disclosure: I was testing automated coding for this merge of stub files and Python files. I’m pleased with the outcome, but don’t want to work on this while I cannot get mypy etc. to run locally. Will wait for the docs.

What do you mean by "cannot get mypy etc. to run locally", exactly?

[ischoegl]

What do you mean by "cannot get mypy etc. to run locally", exactly?

@TimothyEDawson ... I responded here: Cantera/enhancements#251 (comment)

[ischoegl]

Note that this command should report no errors:

mypy --config-file interfaces/cython/pyproject.toml interfaces/cython/cantera

For my setup, I get 172 errors on current main / git hash dff9016

[ischoegl]

@bryanwweber / @TimothyEDawson ... I believe this is now ready for another round. The pyright errors related to pint threw me off, but they are non-blocking and exist on main as well.

[TimothyEDawson]

Awesome, I'll check it out soon! On a related note, I also have finished a first draft of the dev documentation which I'll be pushing soon.