-
Notifications
You must be signed in to change notification settings - Fork 3k
Fix misleading docstring for resolve_matching_names_values #4427
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Fix misleading docstring for resolve_matching_names_values #4427
Conversation
The docstring incorrectly described the behavior of the preserve_order parameter. The descriptions for True and False were swapped. Corrected behavior: - preserve_order=False (default): ordering follows list_of_strings - preserve_order=True: ordering follows the regex keys in data dict Fixes isaac-sim#3849
Greptile OverviewGreptile SummaryThis PR fixes a documentation bug in the Key Changes:
The fix improves code documentation accuracy and resolves issue #3849. Confidence Score: 5/5
Sequence DiagramsequenceDiagram
participant User
participant Function as resolve_matching_names_values
participant CodeLogic as Code Logic
participant Docstring
User->>Function: Calls with preserve_order parameter
Note over Function: Before PR: Docstring had swapped descriptions
alt preserve_order=False (default)
Function->>CodeLogic: Iterate over list_of_strings (lines 323-339)
CodeLogic->>Function: Returns matches in list_of_strings order
Note over Docstring: BEFORE: Incorrectly said "follows regex keys"<br/>AFTER: Correctly says "follows list_of_strings"
else preserve_order=True
Function->>CodeLogic: Reorder by data keys (lines 341-360)
CodeLogic->>Function: Returns matches in regex keys order
Note over Docstring: BEFORE: Incorrectly said "follows list_of_strings"<br/>AFTER: Correctly says "follows regex keys"
end
Function->>User: Returns (indices, names, values)
Note over Function,Docstring: PR fixes docstring to match actual code behavior
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No files reviewed, no comments
Summary
Fix the misleading docstring for
resolve_matching_names_valuesfunction inisaaclab/utils/string.py.The docstring incorrectly described the behavior of the
preserve_orderparameter - the descriptions forTrueandFalsewere swapped.Fixes #3849
Changes
Before (incorrect):
preserve_order=True: ordering follows the list of stringspreserve_order=False: ordering follows the query regular expressionsAfter (correct):
preserve_order=False(default): ordering followslist_of_stringspreserve_order=True: ordering follows the regex keys in data dictionaryVerification
The code logic confirms the corrected behavior:
list_of_stringsin orderpreserve_order=True(lines 341-360): reorders results to followdatakeys orderThe example in the docstring was already correct; only the parameter descriptions were swapped.