Skip to content

Migrate from ligo-segments to igwn-segments #4999

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

Merged
merged 1 commit into from
Jan 7, 2025

Conversation

duncanmmacleod
Copy link
Member

This MR migrates this project from using ligo-segments (which is not supported on new Python versions) with igwn-segments, a fork that is updated and builds on all supported Python versions.

This should have no practical impact on this project or any users.

Standard information about the request

This is a: dependency update

This change affects: the offline search, the live search, inference, PyGRB

This change changes: everything, but has no API change

This change: has appropriate unit tests, follows style guidelines (See e.g. PEP8), has been proposed using the contribution guidelines

This change will: do nothing

Motivation

The ligo-segments project has not been maintained to the point where it doesn't compile for Python 3.12 with new compilers, or for Python 3.13 for any compiler (on all platforms). igwn-segments is a fork that fixes all of the build issues with no API changes.

Contents

All references to ligo-segments and its objects are replaced by the equivalent references to igwn-segments.

Links to any issues or associated PRs

Testing performed

Additional notes

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

Copy link
Contributor

@titodalcanton titodalcanton left a comment

Choose a reason for hiding this comment

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

Long but trivial change, I only have a minor request above.

I would also like @spxiwh and @pannarale to acknowledge this before merging.

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.

Acknowledged as requested!

@pannarale
Copy link
Contributor

Looks like the from igwn_segments import infinity is unused in bin/all_sky_search/pycbc_sngls_findtrigs.
Other than that I have no more comments. Thanks, @duncanmmacleod.

@pannarale
Copy link
Contributor

As a side note, there are other unused imports in this specific script: h5py, pycbc.conversions as conv, trigger_fits imported from pycbc.events as trfits, and indices_outside_times imported from pycbc.events.veto.

@spxiwh
Copy link
Contributor

spxiwh commented Jan 7, 2025

@GarethCabournDavies Please see the comments @pannarale makes here, which are unrelated to this PR, but might be worth addressing separately.

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.

4 participants