Skip to content

Replace direct AUSAXS library integration with the pyAUSAXS wrapper#3634

Merged
butlerpd merged 8 commits intomainfrom
pyausaxs
Oct 21, 2025
Merged

Replace direct AUSAXS library integration with the pyAUSAXS wrapper#3634
butlerpd merged 8 commits intomainfrom
pyausaxs

Conversation

@klytje
Copy link
Contributor

@klytje klytje commented Oct 8, 2025

Description

As discussed in #3559 and #3554, this PR replaces the current direct AUSAXS shared library integration with calls to the new pyAUSAXS wrapper I designed specifically for this integration.

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce.

We should verify that the pyAUSAXS library correctly installs on real systems beyond the GitHub runners. Someone who's more competent than me at building Python wheels should also verify that the three wheels available from pypi are general enough to be installable on all relevant systems.

I have removed the c60.pdb system from Debye testing and relaxed the tolerance to 3% on the remaining systems. Previously, I was compiling higher-precision custom AUSAXS versions for SasView, but pyAUSAXS uses the default, lower-precision release, which was failing these tests.

Review Checklist:

[if using the editor, use [x] in place of [ ] to check a box]

Documentation (check at least one)

  • There is nothing that needs documenting
  • Documentation changes are in this PR
  • There is an issue open for the documentation (link?)

Installers

  • There is a chance this will affect the installers, if so
    • Windows installer (GH artifact) has been tested (installed and worked)
    • MacOSX installer (GH artifact) has been tested (installed and worked)
    • Linux installer (GH artifact) has been tested (installed and worked)
    • Wheels installer (GH artifact) has been tested (installed and worked)

Licensing (untick if necessary)

  • The introduced changes comply with SasView license (BSD 3-Clause)

@klytje klytje marked this pull request as ready for review October 11, 2025 13:22
Copy link
Contributor

@llimeht llimeht left a comment

Choose a reason for hiding this comment

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

Some comments on minor improvements but looks good. Thanks!

@llimeht
Copy link
Contributor

llimeht commented Oct 19, 2025

Thanks @klytje!

Two remaining things:

  • the CI failure on ubuntu-latest will most likely go away if you retry that run. You don't need to fix that transient problem — it's CI break on ubuntu-latest (pycairo fails to install) #3643 and we already have a fix waiting to be merged.
  • there's a merge conflict in this PR so it can't be merged just now. I suspect that this is from the logging work that just got merged. This will need manual work to resolve. My preference is to rebase the changes onto the current main, recrafting the changes as needed; merging the current main into this branch can also work but makes for a messier git history for when we later need to understand who changed what, when, and why.

We should have some Windows and Mac users check that the library is working fine in the installers on those platforms, but if there are issues there, perhaps they just get reported and addressed in subsequent improvement cycles rather than holding up this PR.

@klytje
Copy link
Contributor Author

klytje commented Oct 20, 2025

@llimeht Should be ready to merge now. I've pinned the pyAUSAXS version since I'll probably introduce breaking changes in the next version (supporting #3194). Additionally, I've tested and confirmed the artefacts are working on both Linux & a Windows VM.

@butlerpd butlerpd merged commit 130b354 into main Oct 21, 2025
50 of 52 checks passed
@butlerpd butlerpd deleted the pyausaxs branch October 21, 2025 13:41
@butlerpd
Copy link
Member

@klytje Can we now close issue #3559?

@klytje
Copy link
Contributor Author

klytje commented Oct 21, 2025

@butlerpd Yes I closed it!

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.

3 participants