-
Notifications
You must be signed in to change notification settings - Fork 6
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
feat: update readme with json logger details #47
Conversation
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.
Looks great! Mostly grammatical issues 🚒
marl_eval/json_tools/json_utils.py
Outdated
except zipfile.BadZipFile: | ||
# If the file is not zipped continue to the next file | ||
# as it is already downloaded. | ||
continue |
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.
Don't quite follow why we wouldn't also throw this as an error?
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.
This just assumes the file doesn't have to be unzipped. All files are already downloaded before attempting to unzip them, so we just continue. The zipfile.BadZipFile
is just a bit misleading.
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.
Ah then can we change the docs: saying its already unzipped instead of already downloaded.
Just checking that it's placed in the same directory as the files that are unzipped?
Co-authored-by: Sasha <reallysasha@gmail.com>
Co-authored-by: Sasha <reallysasha@gmail.com>
Co-authored-by: Sasha <reallysasha@gmail.com>
Co-authored-by: Sasha <reallysasha@gmail.com>
Co-authored-by: Sasha <reallysasha@gmail.com>
Co-authored-by: Sasha <reallysasha@gmail.com>
Co-authored-by: Sasha <reallysasha@gmail.com>
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.
Very minor things so approving 😄
marl_eval/json_tools/json_utils.py
Outdated
except zipfile.BadZipFile: | ||
# If the file is not zipped continue to the next file | ||
# as it is already downloaded. | ||
continue |
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.
Ah then can we change the docs: saying its already unzipped instead of already downloaded.
Just checking that it's placed in the same directory as the files that are unzipped?
Co-authored-by: Sasha <reallysasha@gmail.com>
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.
🔢
What?
Add information about the JSON logger to the readme.
Extra
marl_eval/json_tools/__init__.py