Fix: Multiple file summing incorrectly concatenates instead of merging cross-sections#253
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## next #253 +/- ##
==========================================
+ Coverage 65.43% 65.64% +0.20%
==========================================
Files 37 37
Lines 6982 7010 +28
==========================================
+ Hits 4569 4602 +33
+ Misses 2413 2408 -5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
Fixes incorrect behavior when summing multiple REF_M data files by ensuring workspaces are uniquely named per input file, cross-sections are merged (rather than concatenated), and summed workspaces include a run_numbers log so loading reduced/summed runs no longer crashes.
Changes:
- Refactors
Instrument.load_data()to load each file into distinct workspaces, then merge results and add arun_numberssample log. - Updates reduced-file parsing to handle run numbers like
42112+42113and to reconstruct summed file paths when reading saved reductions. - Adds/extends unit tests covering summed-run parsing and multi-file summing behavior; bumps a few dependency minimums in pixi/pyproject.
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
src/quicknxs/interfaces/data_handling/instrument.py |
Refactors multi-file loading to avoid workspace overwrites and sum results; adds run_numbers log. |
src/quicknxs/interfaces/data_handling/quicknxs_io.py |
Improves reduced-file parsing for summed run-number formats and updates file-summing path reconstruction. |
src/quicknxs/interfaces/data_manager.py |
Adjusts additional-peaks initialization to match runs by file path (including summed runs). |
test/unit/quicknxs/interfaces/data_handling/test_instrument.py |
Adds datarepo tests validating multi-file summing and run_numbers presence. |
test/unit/quicknxs/interfaces/data_handling/test_quicknxs_io.py |
Adds tests for summed run parsing and determine_which_files_to_sum() behaviors. |
pyproject.toml |
Updates dependency minimum versions (security-related notes included). |
pixi.lock |
Updates the resolved environment/locks to match new dependency minimums. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
backmari
left a comment
There was a problem hiding this comment.
I have tested in the GUI that the load and sum files functionality now works as expected. Saving and loading of summed files also works as expected. The code changes look good to me.
PR Description
Description of work:
Fixed critical bug in File -> Open & Sum Multiple Files where multiple data files were being concatenated (producing 8 cross-sections from 2 files) instead of properly merged (producing 4 cross-sections). This also fixes the "Unknown property search object 'run_numbers'" error that was crashing the application when loading summed files.
Root Causes Fixed:
_get_xs_list()that never executed due to local variable shadowingxs_list +=) instead of workspace merging inload_data()run_numberslog property on workspacesSolution Implemented:
_get_xs_list()(lines 243-253)load_data()to use unique workspace names for each filecross_section_idusingapi.Plus()run_numberslog with correct format (e.g., "42112+42113")Check all that apply:
References:
(instructions to set up the environment)
After setting up the environment:
Run automated tests:
This should pass, verifying that summing produces 4 cross-sections (not 8) and the
run_numbersproperty exists.Run manual test script:
This validates the fix with real data files (REF_M_42112 and REF_M_42113).
Verify no regressions:
All 10 tests should pass.
Check list for the reviewer