Fix incorrect type annotations by speth · Pull Request #2019 · Cantera/cantera

speth · 2025-10-26T15:04:01Z

Changes proposed in this pull request

Fix some of the new Python type annotations based on (incorrect) errors indicated in the examples and test suite.

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

codecov · 2025-10-26T16:09:41Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 76.47%. Comparing base (3de9745) to head (fa5b17a).
⚠️ Report is 14 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #2019   +/-   ##
=======================================
  Coverage   76.47%   76.47%           
=======================================
  Files         464      464           
  Lines       54936    54938    +2     
  Branches     9305     9305           
=======================================
+ Hits        42012    42014    +2     
  Misses       9798     9798           
  Partials     3126     3126

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

TimothyEDawson · 2025-10-27T00:14:49Z

interfaces/cython/cantera/_types.py

-CompositionLike: TypeAlias = str | dict[str, float] | ArrayLike
-StateSetter: TypeAlias = tuple[float, float, CompositionLike]
+CompositionLike: TypeAlias = str | Mapping[str, float | int] | ArrayLike
+State2Setter: TypeAlias = tuple[float | None, float | None]


I honestly never knew this was an option... You can do stuff like gas.TP = None, 101325.0? What does that do, keep the current temperature as-is? (EDIT: Confirmed that's exactly what it does, TIL!) And if so, why is there no T.setter and P.setter?

Yes, the setters allowing None are a convenient way of changing the state while holding one other property constant. The reason there is no setter for T or P (or H or V or ...) alone is because it's not clear which other thermodynamic property should be held constant.

Of course! That make a lot of sense. Is this feature documented somewhere? It's very convenient, and I'm shocked I never knew about it.

It's mentioned in the Python Tutorial, but perhaps there are a few other places it should be highlighted.

TimothyEDawson · 2025-10-27T00:30:38Z

interfaces/cython/cantera/transport.pyi

 # at https://cantera.org/license.txt for license and copyright information.

-from typing import Literal, TypeAlias, TypedDict
+from typing import Literal, Never, TypeAlias, TypedDict


Need to import Never from typing_extensions instead, to maintain support for Python 3.10. I have some Ruff settings I use which highlight this sort of issue. EDIT: Actually, it was mypy in this case.

Thanks for the pointer. I was glad to see that the error message in the CI job also suggested this change.

Of course! I'm hoping to write up something for the dev guide this week, which will definitely include a recommended way to run the type checking locally so you can be pretty sure the CI won't fail.

For now, you can try these mypy and stubtest commands from main.yaml:

mypy --config-file interfaces/cython/pyproject.toml -p cantera stubtest --mypy-config-file interfaces/cython/pyproject.toml --ignore-disjoint-bases --allowlist interfaces/cython/.mypyignore cantera

Noting that they are both run post-install.

Good to hear. A dev guide for running things locally will be greatly appreciated!

speth · 2025-10-27T03:31:30Z

stubtest is very unhappy with my updates 😢

cantera.ChebyshevRate.__init__ is inconsistent, runtime does not have parameter "temperature_range". You may need to write stubs for __new__ instead of __init__.
cantera.ChebyshevRate.__init__ is inconsistent, runtime does not have parameter "pressure_range". You may need to write stubs for __new__ instead of __init__.
cantera.ChebyshevRate.__init__ is inconsistent, runtime does not have parameter "data". You may need to write stubs for __new__ instead of __init__.
cantera.ChebyshevRate.__init__ is inconsistent, runtime does not have parameter "input_data". You may need to write stubs for __new__ instead of __init__.
cantera.ChebyshevRate.__init__ is inconsistent, runtime does not have parameter "init". You may need to write stubs for __new__ instead of __init__.
cantera.CustomRate.__init__ is inconsistent, runtime does not have parameter "k". You may need to write stubs for __new__ instead of __init__.
cantera.CustomRate.__init__ is inconsistent, runtime does not have parameter "init". You may need to write stubs for __new__ instead of __init__.
cantera.UnitSystem.__init__ is inconsistent, runtime does not have parameter "units". You may need to write stubs for __new__ instead of __init__.
cantera.Units.__init__ is inconsistent, runtime does not have parameter "name". You may need to write stubs for __new__ instead of __init__.
cantera.reaction.ChebyshevRate.__init__ is inconsistent, runtime does not have parameter "temperature_range". You may need to write stubs for __new__ instead of __init__.
cantera.reaction.ChebyshevRate.__init__ is inconsistent, runtime does not have parameter "pressure_range". You may need to write stubs for __new__ instead of __init__.
cantera.reaction.ChebyshevRate.__init__ is inconsistent, runtime does not have parameter "data". You may need to write stubs for __new__ instead of __init__.
cantera.reaction.ChebyshevRate.__init__ is inconsistent, runtime does not have parameter "input_data". You may need to write stubs for __new__ instead of __init__.
cantera.reaction.ChebyshevRate.__init__ is inconsistent, runtime does not have parameter "init". You may need to write stubs for __new__ instead of __init__.
cantera.reaction.CustomRate.__init__ is inconsistent, runtime does not have parameter "k". You may need to write stubs for __new__ instead of __init__.
cantera.reaction.CustomRate.__init__ is inconsistent, runtime does not have parameter "init". You may need to write stubs for __new__ instead of __init__.
cantera.units.UnitSystem.__init__ is inconsistent, runtime does not have parameter "units". You may need to write stubs for __new__ instead of __init__.
cantera.units.Units.__init__ is inconsistent, runtime does not have parameter "name". You may need to write stubs for __new__ instead of __init__.

TimothyEDawson · 2025-10-27T03:32:27Z

interfaces/cython/cantera/reaction.pyi

+        data: ArrayLike | None = None,
+        input_data: _ChebyshevRateInput | None = None,
+        init: bool = True,
+    ) -> None: ...


I'm seeing this is causing an error in the CI:

cantera.ChebyshevRate.__init__ is inconsistent, runtime does not have parameter "temperature_range". You may need to write stubs for __new__ instead of __init__.

This is the same issue I've had with most __cinit__ methods, as their function signature is not passed on to __init__ on the Python side. The way I've been handling it so far is to simply add the specific __init__ to the interfaces/cython/.mypyignore file. Make sure to use the --generate-allowlist option to get the right formatting (it will likely appear twice - once in cantera.ChebyshevRate.__init__ and once in cantera.reaction.ChebyshevRate.__init__).

It's not ideal, but seems to be a limitation of Cython.

TimothyEDawson · 2025-10-27T03:37:31Z

Just left a comment about how to fix it on the ChebyshevRate.__init__. It also makes me realize that the --generate-allowlist command in the CI should print the allowlist so you can easily grab it from the failed CI job.

speth · 2025-10-27T19:28:35Z

Just left a comment about how to fix it on the ChebyshevRate.__init__. It also makes me realize that the --generate-allowlist command in the CI should print the allowlist so you can easily grab it from the failed CI job.

Ah, but there's a bit of a catch-22 with this at present: If the either of the previous stubtest commands fail, the one to generate the allow list is never run (the job runs with bash -e which exits after any failing command).

TimothyEDawson · 2025-10-27T19:34:28Z

Ah, but there's a bit of a catch-22 with this at present: If the either of the previous stubtest commands fail, the one to generate the allow list is never run (the job runs with bash -e which exits after any failing command).

Right, it should probably either be the first command in the three, or better yet, its own separate step. Maybe it should also run without the current allowlist, then print the diff between the result and the current .mypyignore file and/or upload that as a patch file artifact. I just hesitate to overcomplicate things.

ischoegl

Thanks! I’ll go head and approve.

speth added the Python label Oct 26, 2025

TimothyEDawson reviewed Oct 27, 2025

View reviewed changes

speth force-pushed the fix-type-annotations branch from 69335a6 to 894d7e5 Compare October 27, 2025 03:06

TimothyEDawson reviewed Oct 27, 2025

View reviewed changes

speth added 14 commits October 27, 2025 15:47

[Python] Fix type annotations for composition setters

7a1b627

[Python] Fix type annotations for DustyGas properties

01b6b9b

[Python] Fix type annotations for Path as filename

c9eae1c

[Python] Fix key names for derivative settings type hint

f5605f0

[Python] Fix type annotation for Solution slicing

79b4827

[Python] Fix Reactor type annotations

fcf2ae8

[Python] Fix miscellaneous type annotation issues

0efd5c0

[Examples] Show timing info for 1D solver stats

374cedc

[Python] Fix Solution/Interface constructor type annotations

a262bff

[Python] Fix type annotations for partly-constant state setters

95d7cab

[Test] Fix error in HDF restore assertions

6448adf

[Python] Fix type annotations related to unit conversions

eee24f2

[Python] Fix Reaction-related type annotations

e2a9a25

[CI] Ensure updated stubtest allow list is always saved

fa5b17a

speth force-pushed the fix-type-annotations branch from 894d7e5 to fa5b17a Compare October 27, 2025 19:48

speth marked this pull request as ready for review October 27, 2025 21:32

ischoegl approved these changes Oct 28, 2025

View reviewed changes

speth merged commit 6d7b93f into Cantera:main Oct 28, 2025
54 checks passed

speth deleted the fix-type-annotations branch October 28, 2025 02:04

Uh oh!

Conversation

speth commented Oct 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Oct 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

TimothyEDawson Oct 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

speth Oct 27, 2025

Choose a reason for hiding this comment

Uh oh!

TimothyEDawson Oct 27, 2025

Choose a reason for hiding this comment

Uh oh!

speth Oct 27, 2025

Choose a reason for hiding this comment

Uh oh!

TimothyEDawson Oct 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

speth Oct 27, 2025

Choose a reason for hiding this comment

Uh oh!

TimothyEDawson Oct 27, 2025

Choose a reason for hiding this comment

Uh oh!

ischoegl Oct 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

speth commented Oct 27, 2025

Uh oh!

TimothyEDawson Oct 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

TimothyEDawson commented Oct 27, 2025

Uh oh!

speth commented Oct 27, 2025

Uh oh!

TimothyEDawson commented Oct 27, 2025

Uh oh!

ischoegl left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

speth commented Oct 26, 2025 •

edited

Loading

codecov bot commented Oct 26, 2025 •

edited

Loading

TimothyEDawson Oct 27, 2025 •

edited

Loading

TimothyEDawson Oct 27, 2025 •

edited

Loading

ischoegl Oct 27, 2025 •

edited

Loading

TimothyEDawson Oct 27, 2025 •

edited

Loading