Repository navigation
Conversation
…290) prop_type was typed as the Cython enum cea_property_type, so negative values raised OverflowError at the function boundary before the method's own ValueError allowlist check could run. Typed as plain int instead, with explicit casts at the four downstream C-call sites. Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
1 task done
Contributor
Author
1 task done
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes an inconsistency where
Mixture.calc_propertyraised different, unrelated exception types depending on the sign of an invalidprop_type: a positive out-of-range value correctly raisedValueError, but a negative value raisedOverflowErrorwith a Cython-internal message, bypassing the method's own validation entirely.Changes
source/bind/python/CEA.pyx:Mixture.calc_property'sprop_typeparameter changed from the Cython enum typecea_property_typeto a plainint. This removes Cython's automatic enum-bounds coercion at the function boundary (the actual source of the strayOverflowError), so every invalid value — positive or negative — now falls through uniformly to the method's existingif prop_type not in [...]: raise ValueError(...)allowlist check.cea_mixture_calc_property,cea_mixture_calc_property_tp, and their_multitempvariants) still require thecea_property_typeC enum per their fixed signatures incea_def.pxd, so each call site now has an explicit<cea_property_type>prop_typecast. This is a compile-time C-level cast with no runtime coercion, and by the time it runsprop_typehas already passed the allowlist check, so it never sees an invalid value.source/bind/python/tests/test_mixture.py:test_calc_property_rejects_unknown_typeis now parametrized over both999999(pre-existing positive case) and-5, assertingValueErrorfor both.Testing
make py-rebuildpytest source/bind/python/tests -v— 114 passedValueError: Property type not supported for mixture calculationsinstead of one raisingOverflowErrorCompatibility / Numerical behavior
This only changes exception-raising behavior for already-invalid
prop_typeinputs; no valid-input code path or numerical calculation changes.Drafted with Claude's assistance
CEA.pyx'scalc_propertymethod andcea_def.pxd's C-call signatures directly to trace why the enum-typed parameter caused the coercion, and why the downstream calls needed explicit casts🤖 Generated with Claude Code