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

Segment tags #4468

Merged
merged 9 commits into from
Aug 30, 2023
Merged

Segment tags #4468

merged 9 commits into from
Aug 30, 2023

Conversation

maxtrevor
Copy link
Contributor

Allows for segments to be read from different locations in one search by using config tags

@maxtrevor maxtrevor requested a review from spxiwh August 29, 2023 00:47
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.

Thanks Max. This largely looks good, but some minor requests made here.

Also as a style thing, when a function uses tags as a key-word argument, I prefer it if that function is called with tags=tags, rather than just tags. This avoids issues if we change the function in the future.

Also one line that maybe could be wrapped to 80 characters pointed out by CodeClimate.

bin/workflows/pycbc_make_offline_search_workflow Outdated Show resolved Hide resolved
bin/workflows/pycbc_make_offline_search_workflow Outdated Show resolved Hide resolved
bin/workflows/pycbc_make_offline_search_workflow Outdated Show resolved Hide resolved
pycbc/workflow/segment.py Show resolved Hide resolved
pycbc/workflow/segment.py Outdated Show resolved Hide resolved
pycbc/workflow/segment.py Show resolved Hide resolved
pycbc/workflow/segment.py Outdated Show resolved Hide resolved
@maxtrevor
Copy link
Contributor Author

@spxiwh this should be good to go now

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.

Great, thanks. Green light from me once the automated tests, including the workflow tests, complete.

@maxtrevor
Copy link
Contributor Author

maxtrevor commented Aug 29, 2023

@spxiwh I added one more commit so that the segments workflow can parse a dq flag provided in the [workflow-segments] config section without needing it to be tagged to a specific IFO. I also fixed a weird indentation issue.

@spxiwh spxiwh merged commit 6c2bfd3 into gwastro:master Aug 30, 2023
36 checks passed
GarethCabournDavies pushed a commit to GarethCabournDavies/pycbc that referenced this pull request Oct 11, 2023
* Added support for dq tags

* Small fix

* Addressed some of Ian's comments

* Fixed indent mistake

* Added description of tags argument to docstring

* Small fix to tags

* Don't use tags with veto-definer

* Don't require dq segments to be set for each ifo separately

* Fixed over-indent
PRAVEEN-mnl pushed a commit to PRAVEEN-mnl/pycbc that referenced this pull request Nov 3, 2023
* Added support for dq tags

* Small fix

* Addressed some of Ian's comments

* Fixed indent mistake

* Added description of tags argument to docstring

* Small fix to tags

* Don't use tags with veto-definer

* Don't require dq segments to be set for each ifo separately

* Fixed over-indent
bhooshan-gadre pushed a commit to bhooshan-gadre/pycbc that referenced this pull request Mar 4, 2024
* Added support for dq tags

* Small fix

* Addressed some of Ian's comments

* Fixed indent mistake

* Added description of tags argument to docstring

* Small fix to tags

* Don't use tags with veto-definer

* Don't require dq segments to be set for each ifo separately

* Fixed over-indent
acorreia61201 pushed a commit to acorreia61201/pycbc that referenced this pull request Apr 4, 2024
* Added support for dq tags

* Small fix

* Addressed some of Ian's comments

* Fixed indent mistake

* Added description of tags argument to docstring

* Small fix to tags

* Don't use tags with veto-definer

* Don't require dq segments to be set for each ifo separately

* Fixed over-indent
@maxtrevor maxtrevor deleted the dq_tags branch May 1, 2024 19:08
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