Allow substitutions for ParameterFile allow_substs - #552
Open
Old-Ding wants to merge 2 commits into
Open
Conversation
Accept the substitution types already normalized by ParameterFile so XML and YAML frontends can defer allow_substs evaluation until runtime. Signed-off-by: Old-Ding <35417409+Old-Ding@users.noreply.github.com>
|
Tick the box to add this pull request to the merge queue (same as
|
Old-Ding
force-pushed
the
codex/parameter-file-allow-substs
branch
2 times, most recently
from
July 12, 2026 18:07
1ffda44 to
6146d2b
Compare
Store the resolved boolean after successful evaluation so the public property reports the effective value. Reset dynamic substitution caches and temporary-file ownership during cleanup so later contexts are re-evaluated without exposing the source parameter file to deletion. Initialize cleanup state before validation so rejected arguments cannot trigger a secondary unraisable exception during destruction. Signed-off-by: Old-Ding <35417409+Old-Ding@users.noreply.github.com>
Old-Ding
force-pushed
the
codex/parameter-file-allow-substs
branch
from
July 12, 2026 19:01
6146d2b to
673aa58
Compare
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
1 participant
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.
Description
ParameterFiledeclares and normalizesallow_substsas a boolean or substitution, and the node frontend parses substitution-valued attributes before constructing it. However, its constructor validation only acceptedbool, so both Python substitutions and XML/YAML substitution lists raisedTypeError.Accept the existing
SomeSubstitutionsTypevariants in that validation, exercise both node frontends with a runtime expression, and cache the resolved boolean after successful evaluation. Cleanup resets dynamic substitution caches and temporary-file ownership so a reused description evaluates the new context and never treats the original parameter file as a generated temporary file.Is this user-facing behavior change?
Yes.
allow_substscan now be selected by a launch substitution in Python, XML, and YAML as its API already declares, instead of failing while constructingParameterFile. After evaluation, theallow_substsproperty reports the resolved boolean; after cleanup, a later context is evaluated again.Did you use Generative AI?
Yes. OpenAI Codex assisted with root-cause analysis and preparing the focused code and tests. I reviewed the diff and verification results.
Additional Information
Local verification:
ParameterFiletests: 8 passed, including invalid-constructor cleanupbool,LaunchConfiguration, frontend-style substitution lists, and invalid objectsament_flake8,ament_copyright, andament_pep257for all three changed filesgit diff --checkThe full node frontend test requires a ROS 2 runtime with
rclpy,lifecycle_msgs, anddemo_nodes_py, which is not installed on this Windows host; repository CI covers that integration test.xmllintwas also unavailable locally.