[Infra]: fix run_vtr_task lint error #1821
Conversation
| @@ -227,7 +227,7 @@ def vtr_command_main(arg_list, prog=None): | |||
|
|
|||
| config_files = [find_task_config_file(task_name) for task_name in task_names] | |||
| configs = [] | |||
| common_task_prefix = None # common task prefix to shorten task names | |||
| common_task_prefix = "" # common task prefix to shorten task names | |||
There was a problem hiding this comment.
It looks like this changes the functionality of the script. Shouldn't common_task_prefix be initialized to config.task_name instead of ""? And with that change, I think the if / else checking if common_task_prefix is None can be deleted (just always do the else).
There was a problem hiding this comment.
I think the definition of common_task_prefix initializes by empty string needs to be outside of the for loop since the 'shorten_task_names' function uses this variable. By that, I mean the value of config.task_name is specified inside the loop so we can not initialize the common_task_prefix with the value of config.task_name outside of the loop. However, you are right about the if / else expression, the if expression should change to 'if not common_task_prefix' to follow the previous path. Sorry for this mistake
There was a problem hiding this comment.
Please correct me if I am wrong
There was a problem hiding this comment.
Ah, you're right -- didn't read that carefully enough. Yes, I think the fix will work.
|
@sdamghan : I think the script functionality is changed by this PR, but there is an alternative change that wouldn't change it (and should simplify the logic). Please take a look. |
Signed-off-by: Seyed Alireza Damghani <[email protected]>
Signed-off-by: Seyed Alireza Damghani [email protected]
Description
specifying the
common_task_prefixvariable type while initializing itRelated Issue
Motivation and Context
fix CI error for python-format
How Has This Been Tested?
Types of changes
Checklist: