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

Move to igwn-ligolw #5019

Draft
wants to merge 11 commits into
base: master
Choose a base branch
from

Conversation

titodalcanton
Copy link
Contributor

@titodalcanton titodalcanton commented Jan 22, 2025

Move from python-ligo-lw to the recently released and more modern igwn-ligolw.

Standard information about the request

This is a depencency change.
This change should have no impact on any functionality or result.
This change will drop the dependency on python-ligo-lw and introduce a depencency on igwn-ligolw.

Motivation

igwn-ligolw is more actively maintained and supports modern Python versions and more architectures than python-ligo-lw.

Contents

Mainly replacing imports, but the transition from python-ligo-lw 1.8.3 to igwn-ligolw 2.0.0 has the side effect of introducing a few breaking changes which never made it to a python-ligo-lw release yet, as well as a couple deprecation warnings, so those are handled as well. We can also further simplify tox.ini, consistently with #4857.

Links to any issues or associated PRs

This is essentially a counterpart to #4999. Improves #4857.

Testing performed

Ran tox and a few of the tests manually on a local machine, but I am relying on the CI for now.

Additional notes

I am not sure I want to push this to production codes yet, at least not before more significant testing is done with workflows and GraceDB interactions. In particular, I think we should create a release branch for PyGRB's O4a analysis before merging this (@pannarale). So I am applying a "on hold" label.

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

@titodalcanton titodalcanton marked this pull request as draft January 22, 2025 11:18
@titodalcanton titodalcanton added on hold dependencies Pull requests that update a dependency file labels Jan 22, 2025
@titodalcanton
Copy link
Contributor Author

titodalcanton commented Feb 5, 2025

Started to do some more thorough tests. Noting down a few more deprecations to address:

UserWarning: igwn_ligolw.utils.process.register_to_xmldoc() is deprecated.  use igwn_ligolw.ligolw.Document.register_process() instead.
UserWarning: igwn_ligolw.lsctables.New() is deprecated.  use igwn_ligolw.Table.new() instead.
UserWarning: igwn_ligolw.lsctables.use_in() is deprecated.  simply importing the lsctables module automatically enables the custom table support features it provides.

Edit: these have been addressed now.

@titodalcanton
Copy link
Contributor Author

I made a test run of the PyCBC Live example uploading to GraceDB test and I do not see particular issues, so I am gaining confidence about this.
image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file on hold
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant