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

support python3.12 now, so use bbhx master #4834

Merged
merged 8 commits into from
Jul 31, 2024

Conversation

WuShichao
Copy link
Contributor

Standard information about the request

This is a: plugin update

This change affects: inference, LISA-related

This change changes: documentation, result presentation / plotting, scientific output

This change will: almost no changes

Motivation

Supports the latest BBHx version.

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

tox.ini Outdated Show resolved Hide resolved
@GarethCabournDavies
Copy link
Contributor

obviously there have been a lot of changes to CI on master since this

so this can be either closed so it can be developed, or repurposed for experimenting with setting up the BBHx in CI again

@titodalcanton
Copy link
Contributor

On a quick inspection of BBHx, I am not convinced that it functionally requires Python 3.12. With a minor edit I managed to install it on a 3.9 virtualenv, though I have not tested it. My change is on this branch:

https://github.com/titodalcanton/BBHx/tree/py39-and-cleanup

@WuShichao could you try using that branch instead of master here, re-enabling the LISA tests that were disabled in #4833, and see if they pass?

@WuShichao
Copy link
Contributor Author

Reopen with Tito's BBHx.

@WuShichao WuShichao reopened this Jul 31, 2024
tox.ini Outdated Show resolved Hide resolved
tox.ini Outdated Show resolved Hide resolved
@titodalcanton
Copy link
Contributor

Do not forget to reenable the tests, i.e. basically revert #4833 apart from the change to setup.py.

@WuShichao WuShichao changed the title support python12 now, so use bbhx master support python3.12 now, so use bbhx master Jul 31, 2024
@titodalcanton
Copy link
Contributor

Ok, progress… @GarethCabournDavies @WuShichao I will let you decide if you want to merge this as-is now, or get BBHx to correct its Python requirements upstream.

@WuShichao
Copy link
Contributor Author

Ok, progress… @GarethCabournDavies @WuShichao I will let you decide if you want to merge this as-is now, or get BBHx to correct its Python requirements upstream.

The current solution seems work, we can merge this for CI tests and doc, fix from upstream will take some time.

@titodalcanton titodalcanton merged commit 63c0ecf into gwastro:master Jul 31, 2024
30 checks passed
@GarethCabournDavies
Copy link
Contributor

Just getting back to it, thanks both of you for sorting this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants