Skip to content

Improve the compute score script to accept json directory or json file without requiring a config. - #144

Merged
xuzhao9 merged 6 commits into
gh/xuzhao9/1/basefrom
gh/xuzhao9/1/head
Oct 26, 2020
Merged

Improve the compute score script to accept json directory or json file without requiring a config.#144
xuzhao9 merged 6 commits into
gh/xuzhao9/1/basefrom
gh/xuzhao9/1/head

Conversation

@xuzhao9

@xuzhao9 xuzhao9 commented Oct 23, 2020

Copy link
Copy Markdown
Contributor

Stack from ghstack:

… json file without requiring a config.

[ghstack-poisoned]
xuzhao9 added a commit that referenced this pull request Oct 23, 2020
… json file without requiring a config.

ghstack-source-id: 69a5195
Pull Request resolved: #144
…irectory or json file without requiring a config."

[ghstack-poisoned]
xuzhao9 added a commit that referenced this pull request Oct 23, 2020
… json file without requiring a config.

ghstack-source-id: 79c0b76
Pull Request resolved: #144
@xuzhao9
xuzhao9 requested a review from wconstab October 26, 2020 18:08
Comment thread score/compute_score.py
with open(args.configuration) as cfg_file:
config = yaml.full_load(cfg_file)

if args.configuration is not None:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

does it make sense to reduce lines of code by putting the config-file generation into the existing code blocks
if args.benchmark_data_file is not None:
elif args.benchmark_data_dir is not None:
?

You can still have a check at line 53
else:
if args.benchmark_data_file is None and args.benchmark_data_file is None, raise Exception "invalid arg.."

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

That makes sense. I have updated the code.

@wconstab wconstab left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM! minor nit comment inline

…irectory or json file without requiring a config."

[ghstack-poisoned]
xuzhao9 added a commit that referenced this pull request Oct 26, 2020
… json file without requiring a config.

ghstack-source-id: 1f301fd
Pull Request resolved: #144
…irectory or json file without requiring a config."

[ghstack-poisoned]
xuzhao9 added a commit that referenced this pull request Oct 26, 2020
… json file without requiring a config.

ghstack-source-id: cec51f6
Pull Request resolved: #144
…irectory or json file without requiring a config."

[ghstack-poisoned]
xuzhao9 added a commit that referenced this pull request Oct 26, 2020
… json file without requiring a config.

ghstack-source-id: 2ef6c4d
Pull Request resolved: #144
…irectory or json file without requiring a config."

[ghstack-poisoned]
xuzhao9 added a commit that referenced this pull request Oct 26, 2020
… json file without requiring a config.

ghstack-source-id: cec51f6
Pull Request resolved: #144
@xuzhao9 xuzhao9 changed the title Improve the compute score script so that it accepts json directory or json file without requiring a config. Improve the compute score script to accept json directory or json file without requiring a config. Oct 26, 2020
@xuzhao9
xuzhao9 merged commit 979c6f4 into gh/xuzhao9/1/base Oct 26, 2020
@xuzhao9
xuzhao9 deleted the gh/xuzhao9/1/head branch October 26, 2020 23:47
wconstab pushed a commit that referenced this pull request Oct 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants