cctbx / cctbx_project

Computational Crystallography Toolbox
https://cci.lbl.gov/docs/cctbx
Other
210 stars 111 forks source link

Break the DIALS/xfel circular dependency #872

Closed phyy-nx closed 9 months ago

phyy-nx commented 1 year ago

DIALS and dxtbx have had dependencies on cctbx_project/xfel, and vice versa. This PR is one of 8 (!) that will all be merged in simultaneously to 8 repositories to break the cyclic dependency.

Brief summary:

All tests pass on my local test machine (DIALS, dxtbx, and XFEL CI).

Se also cctbx/dxtbx#627 and dials/dials#2404

Detailed listing of code movements:

From xfel.cftbx.detector:

From xfel.mono_simulation:

From xfel.cftbx.detector.cspad_cbf_tbx:

From xfel.command_line.cspad_detector_congruence:

From xfel.command_line.frame_extractor:

From xfel.cxi.cspad_ana.cspad_tbx:

From xfel.cxi.cspad_ana.rayonix_tbx.py:

C++ code from xfel/ext.cpp:

From xfel.util:

phyy-nx commented 1 year ago

How 'bout that. The tests all passed :)

This is ready for review!

phyy-nx commented 11 months ago

/azp run

azure-pipelines[bot] commented 11 months ago
Azure Pipelines successfully started running 3 pipeline(s).
phyy-nx commented 10 months ago

Test expected to fail right now as they depend on the dials and dxtbx PRs (cctbx/dxtbx#627 and dials/dials#2404).

ndevenish commented 10 months ago

Minor thought: do you think it would be worth doing in two phases, adding the serialtbx sections searately, before updating xfel? It would save any sticky disentanglement if things go wrong up/downstream.

phyy-nx commented 10 months ago

Ah, you mean add the serialtbx changes only, and do code removal from xfel in a separate pass? Not a bad way to go, but I think the testing should have caught any problems. I'll run the tests again today to be sure.