Skip to content

Add special cases to coerce "1" and "0" to bool when using bool coercion only#651

Merged
methylDragon merged 3 commits intorollingfrom
ch3/coerce-int-str-to-bool
Sep 19, 2022
Merged

Add special cases to coerce "1" and "0" to bool when using bool coercion only#651
methylDragon merged 3 commits intorollingfrom
ch3/coerce-int-str-to-bool

Conversation

@methylDragon
Copy link
Copy Markdown
Contributor

@methylDragon methylDragon commented Sep 16, 2022

Description

Exactly as the title says.

Given that "1" and "0" are valid true and false expressions according to the conditions, I think it makes sense to allow coercion of the "1" and "0" strings to True and False.

This ONLY affects coercion when bool is explicitly passed to it. It's as small a change as possible.

This supports the work in #649 and will be used to support the boolean substitution implementations.

Tests

Special note: This means that one test for the type coercion will no longer apply.

Beyond that, relevant tests have been implemented.

Signed-off-by: methylDragon <methylDragon@gmail.com>
Signed-off-by: methylDragon <methylDragon@gmail.com>
@methylDragon methylDragon force-pushed the ch3/coerce-int-str-to-bool branch from d9e3d43 to bfb744c Compare September 16, 2022 21:16
@methylDragon
Copy link
Copy Markdown
Contributor Author

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Windows Build Status

@methylDragon methylDragon marked this pull request as draft September 16, 2022 21:23
@methylDragon methylDragon force-pushed the ch3/coerce-int-str-to-bool branch from 60b0377 to bfb744c Compare September 16, 2022 21:44
@methylDragon methylDragon changed the title Add special cases to coerce "1" and "0" to bool Add special cases to coerce "1" and "0" to bool when using bool coercion only Sep 16, 2022
@methylDragon methylDragon marked this pull request as ready for review September 17, 2022 00:02
Copy link
Copy Markdown
Member

@wjwwood wjwwood left a comment

Choose a reason for hiding this comment

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

Makes sense to me, but I think @ivanpauno or @hidmic need to have a look, since they worked on this originally (I think). The coercion stuff is kind of tricky, so I could be missing something.

Copy link
Copy Markdown
Member

@jacobperron jacobperron left a comment

Choose a reason for hiding this comment

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

+1 from me

Copy link
Copy Markdown
Member

@ivanpauno ivanpauno left a comment

Choose a reason for hiding this comment

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

lgtm

Signed-off-by: methylDragon <methylDragon@gmail.com>
@methylDragon methylDragon merged commit 9e517a3 into rolling Sep 19, 2022
@methylDragon methylDragon deleted the ch3/coerce-int-str-to-bool branch September 19, 2022 18:32
emersonknapp pushed a commit to emersonknapp/launch that referenced this pull request May 1, 2025
emersonknapp pushed a commit to emersonknapp/launch that referenced this pull request May 1, 2025
…ion only (ros2#651)

Signed-off-by: Emerson Knapp <emerson.b.knapp@gmail.com>
ahcorde pushed a commit that referenced this pull request May 6, 2025
…ion only (#651) (#872)

Signed-off-by: Emerson Knapp <emerson.b.knapp@gmail.com>
Co-authored-by: methylDragon <methylDragon@gmail.com>
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.

4 participants