Fix misleading docstring for resolve_matching_names_values#4427
Open
Wheeeeeeeeels wants to merge 5 commits intoisaac-sim:mainfrom
Open
Fix misleading docstring for resolve_matching_names_values#4427Wheeeeeeeeels wants to merge 5 commits intoisaac-sim:mainfrom
Wheeeeeeeeels wants to merge 5 commits intoisaac-sim:mainfrom
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
Contributor
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
|
2ef7fc8 to
f3061a4
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
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.
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.