Skip to content

Comments

fix: correct inverted preserve_order docstring in resolve_matching_names#4678

Open
alltheseas wants to merge 1 commit intoisaac-sim:mainfrom
alltheseas:fix/resolve-matching-names-docstring
Open

fix: correct inverted preserve_order docstring in resolve_matching_names#4678
alltheseas wants to merge 1 commit intoisaac-sim:mainfrom
alltheseas:fix/resolve-matching-names-docstring

Conversation

@alltheseas
Copy link

Summary

Fix the inverted preserve_order parameter description in the resolve_matching_names docstring.

The docstring incorrectly stated that preserve_order=True returns results in list_of_strings order, and preserve_order=False returns in query-key order. The actual behavior is the opposite:

  • preserve_order=False (default): results follow list_of_strings order
  • preserve_order=True: results are reordered to follow query regex key order

This is the same bug that PR #4427 fixes for the sibling function resolve_matching_names_values. This PR addresses the remaining function noted in #4493.

Fixes #4493

Changes

Signed-off-by: alltheseas alltheseas@users.noreply.github.com

The docstring for `resolve_matching_names` incorrectly described the
behavior of the `preserve_order` parameter — the descriptions for
True and False were swapped.

Fixes isaac-sim#4493

Signed-off-by: alltheseas <alltheseas@users.noreply.github.com>
@github-actions github-actions bot added bug Something isn't working isaac-lab Related to Isaac Lab team labels Feb 21, 2026
@greptile-apps
Copy link
Contributor

greptile-apps bot commented Feb 21, 2026

Greptile Summary

This PR corrects the inverted preserve_order parameter documentation in the resolve_matching_names function. The docstring previously had the True/False descriptions swapped - it incorrectly stated that preserve_order=True returns results in list_of_strings order and preserve_order=False returns in query-key order, when the actual behavior is the opposite.

Key changes:

  • Swapped the True/False descriptions to match actual implementation behavior
  • Added (default) annotation to clarify that False is the default value
  • Added inline clarifications to the code examples for better understanding

The fix is verified by examining the implementation logic on lines 242-259, which shows that when preserve_order=True, the function reorders results to follow the query regex key order, not the list_of_strings order.

Confidence Score: 5/5

  • This PR is completely safe to merge - it only corrects documentation without any code changes
  • This is a pure documentation fix that corrects inverted parameter descriptions in a docstring. No functional code is changed, so there is zero risk of introducing bugs or breaking changes. The fix has been verified against the actual implementation logic.
  • No files require special attention

Important Files Changed

Filename Overview
source/isaaclab/isaaclab/utils/string.py Fixes inverted preserve_order parameter documentation in resolve_matching_names function - docstring now correctly describes the actual behavior

Last reviewed commit: b4c20a0

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

1 file reviewed, no comments

Edit Code Review Agent Settings | Greptile

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

Labels

bug Something isn't working isaac-lab Related to Isaac Lab team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Docstring error in resolve_matching_names and resolve_matching_names_values: preserve_order description is inverted

1 participant