Skip to content

Treat bare YAML nan and inf as strings - #1323

Open
Old-Ding wants to merge 1 commit into
ros2:rollingfrom
Old-Ding:codex/yaml-bare-nonfinite-strings
Open

Treat bare YAML nan and inf as strings#1323
Old-Ding wants to merge 1 commit into
ros2:rollingfrom
Old-Ding:codex/yaml-bare-nonfinite-strings

Conversation

@Old-Ding

@Old-Ding Old-Ding commented Jul 11, 2026

Copy link
Copy Markdown

Description

rcl_yaml_param_parser explicitly handles the YAML 1.1 dot-prefixed non-finite forms such as .nan and .inf. Its generic strtod() fallback also accepted untagged bare nan, inf, +inf, and -inf, even though YAML treats those forms as strings.

Reject non-finite results only during implicit type inference, leaving both the existing dot-prefixed forms and explicitly tagged values such as !!float nan unchanged. Extend the special-float integration fixture to verify the four implicit bare forms remain strings and the four explicitly tagged forms remain doubles.

Fixes #1320.

Is this user-facing behavior change?

Yes. Untagged bare nan and inf parameter values are now strings, matching YAML and the composable-node parameter loading path. Valid dot-prefixed and explicit !!float non-finite values continue to be parsed as doubles.

Did you use Generative AI?

Yes. OpenAI Codex assisted with root-cause analysis and preparing the focused code and regression test. I reviewed the diff and verification results.

Additional Information

Local verification:

  • a native C probe verified implicit bare non-finite values are rejected as floats, explicit-tag mode accepts them, dot-prefixed forms remain floats, and finite values are unchanged
  • PyYAML verification confirmed the fixture has 11 string entries and four explicitly tagged non-finite doubles
  • source invariants confirm only the explicit YAML_FLOAT_TAG path enables generic non-finite parsing
  • all changed lines are at most 100 characters
  • UTF-8/CRLF and git diff --check

The full rcl_yaml_param_parser gtest requires ROS 2 build dependencies that are unavailable on this Windows host; repository CI covers the integration test.

@mergify

mergify Bot commented Jul 11, 2026

Copy link
Copy Markdown

Tick the box to add this pull request to the merge queue (same as @mergifyio queue).

  • Queue this pull request

Limit implicit non-finite float parsing to the YAML dot-prefixed forms so standalone parameter loading matches the composable-node path. Preserve generic non-finite forms when an explicit !!float tag requests them.

Signed-off-by: Old-Ding <35417409+Old-Ding@users.noreply.github.com>
@Old-Ding
Old-Ding force-pushed the codex/yaml-bare-nonfinite-strings branch from d968fb5 to 0bcf6ec Compare July 12, 2026 18:47

@fujitatomoya fujitatomoya left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@Old-Ding the fix itself looks good to me.

seems like you are creating bunch of PRs in a few days.
please have a close look at https://discourse.openrobotics.org/t/guidelines-for-ai-assisted-contributions-in-the-ros-project/55903.

and can you provide the test result for this? either log or screen capture?

@fujitatomoya

Copy link
Copy Markdown
Collaborator

@Old-Ding the fix itself looks good to me.

seems like you are creating bunch of PRs in a few days. please have a close look at https://discourse.openrobotics.org/t/guidelines-for-ai-assisted-contributions-in-the-ros-project/55903.

and can you provide the test result for this? either log or screen capture?

@Old-Ding friendly ping.

@Old-Ding

Copy link
Copy Markdown
Author

Verification result for the current head: https://github.com/Old-Ding/rcl/actions/runs/29411533231

I ran the target package in an Ubuntu ROS Rolling container:

  • colcon build --packages-select rcl_yaml_param_parser
  • colcon test --packages-select rcl_yaml_param_parser
  • test_parse_yaml passed, including the updated special_float_point coverage.
  • colcon test-result --verbose: 183 tests, 0 errors, 0 failures, 32 skipped.

The full build and test log is linked above.

@Old-Ding

Copy link
Copy Markdown
Author

@fujitatomoya Thank you for the reminder. I have read the AI-assisted contribution guidelines. This PR description discloses the use of Codex, and I reviewed the parser paths and the regression coverage before submitting it.

Verification for the current PR source is here: https://github.com/Old-Ding/rcl/actions/runs/29411533231

  • Built and tested rcl_yaml_param_parser in an Ubuntu ROS Rolling container.
  • test_parse_yaml passed, including the updated special_float_point case.
  • colcon test-result --verbose: 183 tests, 0 errors, 0 failures, 32 skipped.

The workflow branch contains only its verification workflow in addition to PR head 0bcf6ec; the source tree under test is otherwise identical. I will continue to keep submissions narrow and include reproducible focused test evidence with them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

rcl_yaml_param_parser treats bare nan/inf as doubles via fallback (inconsistent with composable node parameter loading)

2 participants