Skip to content
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

Merged
merged 2 commits into from
Sep 20, 2024

Conversation

pannarale
Copy link
Contributor

@pannarale pannarale commented Sep 19, 2024

This PR follows up on comments received in, e.g., #4877.

It simply removes the other calls to resolve_url_to_file in grb_utils.py and some remaining commented lines of code. It also fixes a typo in a comment in pycbc_multi_inspiral.

This change affects: PyGRB

It was tested in producing this webpage.

  • The author of this pull request confirms they will adhere to the code of conduct

@pannarale pannarale added the PyGRB PyGRB development label Sep 19, 2024
@pannarale pannarale self-assigned this Sep 19, 2024
Copy link
Contributor

@spxiwh spxiwh left a 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"]
Copy link
Contributor

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

Copy link
Contributor

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.

Copy link
Contributor Author

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!

@pannarale pannarale merged commit dc2f584 into gwastro:master Sep 20, 2024
30 checks passed
@pannarale pannarale deleted the further_grb_utils_cleanup branch September 20, 2024 09:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PyGRB PyGRB development
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants