Skip to content

[Infra]: fix run_vtr_task lint error #1821

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Aug 12, 2021
Merged

[Infra]: fix run_vtr_task lint error #1821

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions vtr_flow/scripts/run_vtr_task.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -227,11 +227,11 @@ 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
Copy link
Contributor

Choose a reason for hiding this comment

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

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).

Copy link
Member Author

Choose a reason for hiding this comment

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

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

Copy link
Member Author

Choose a reason for hiding this comment

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

Please correct me if I am wrong

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, you're right -- didn't read that carefully enough. Yes, I think the fix will work.

for config_file in config_files:
config = load_task_config(config_file)
configs += [config]
if common_task_prefix is None:
if not common_task_prefix:
common_task_prefix = config.task_name
else:
match = SequenceMatcher(
Expand Down