-
Notifications
You must be signed in to change notification settings - Fork 357
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
Further grb utils cleanup #4886
Further grb utils cleanup #4886
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.
LGTM. One suggestion for the future below, but happy for this to be merged as is.
"""Construct a FileList instance containing all segments txt files""" | ||
|
||
seg_dir = workflow.cp.get('workflow', 'segment-dir') | ||
file_names = ["bufferSeg.txt", "offSourceSeg.txt", "onSourceSeg.txt"] |
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.
A suggestion here to combine these 3 files into one (these were horrible even in 2011).
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.
... Probably not for this PR though.
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.
I opened an issue #4887 for future PyGRB development. Thanks!
* Removing more resolve_url_to_file calls and commented lines * Fixing typo in comment
This PR follows up on comments received in, e.g., #4877.
It simply removes the other calls to
resolve_url_to_file
ingrb_utils.py
and some remaining commented lines of code. It also fixes a typo in a comment inpycbc_multi_inspiral
.This change affects: PyGRB
It was tested in producing this webpage.