Skip to content

Add ability to add non-"Direct Beam" PV-labeled runs to the Direct Beam table - #229

Closed
darshdinger wants to merge 20 commits into
nextfrom
ewm11847-add-ability-to-load-non-Direct_beam-labeled-pv-data
Closed

darshdinger wants to merge 20 commits into
nextfrom
ewm11847-add-ability-to-load-non-Direct_beam-labeled-pv-data

Conversation

@darshdinger

@darshdinger darshdinger commented Nov 11, 2025

Copy link
Copy Markdown
Contributor

Description of work:

Allow users to add any run to the Direct Beam table even if the EPICS PV data_type is not set to indicate a direct beam (data_type != 1). This enables calibration and other runs started with EPICS "Start RUN" (which may not set the Direct Beam PV-tag) to be used as normalization runs.

Key changes:

  • DataManager.add_active_to_direct_beam_list() now accepts any run and returns an integer status:
    • 2 — added and is a true direct beam (data_type == 1)
    • 1 — added but not labeled as a direct beam (data_type != 1); logs a warning
    • 0 — not added (already present)
  • MainHandler.add_direct_beam() updated to show a non-blocking warning message when a non-direct-beam run is added (keeps blocking error for duplicates).
  • Kept is_direct_beam metadata unchanged to preserve original metadata integrity and avoid altering run semantics.
  • PlotManager.plot_reflectivity_or_intensity() updated to check if active data is in the direct beam list (in addition to checking is_direct_beam attribute) to correctly display "Intensity" plots for non-direct-beam runs added to the direct beam table.
  • Note: Users can intentionally add the same run to both the Direct Beam table and the Data/Reduction table. This is a supported use case for comparing different ROIs (e.g., portion of beam that misses the sample vs. reflected beam on the same detector). When a run exists in the direct beam list, it will display as "Intensity" regardless of also being in the reduction table.
  • Added comprehensive tests to cover behavior and backward compatibility:
    • Unit tests using real data (marked @pytest.mark.datarepo)
    • Mock-based unit tests that don't need data files
    • UI integration tests that confirm warnings, duplicate handling, normalization behavior, and plot display

Check all that apply:

  • updated documentation and checked that it looks correct in the pull request preview
  • Source added/refactored
  • Added unit tests
  • Added integration tests

References:

  • Links to IBM EWM items: EWM 11847
  • Links to related issues or pull requests: (none)

⚠️ Manual test for the reviewer

(Quick steps to exercise the feature locally; CI may need the data repo for full integration tests.)

  1. Set up the environment:
    • Ensure dependencies installed (pixi env used in this repo).
  2. Run non-datarepo unit tests (fast smoke test):
    pixi run pytest test/unit/quicknxs/interfaces/test_data_manager_direct_beam_mock.py -v
  3. Run UI tests or datarepo tests only if you have the SNS data available:
    # run UI tests (requires data Server fixture / datarepo)
    pixi run pytest test/ui/test_add_non_direct_beam.py -v
    
    # Or run the specific integration tests marked datarepo:
    pixi run pytest -m datarepo test/unit/quicknxs/interfaces/test_data_manager_direct_beam.py -v
  4. Manual interactive check:
    • Start the GUI (if you normally launch QuickNXS).
    • Load a scattering run (a run whose data_type PV is not 1).
    • Use "Add Direct Beam" (Ctrl+W or Reduction → Add Direct Beam) — confirm the run is added to the Direct Beam table and a non-blocking warning is shown in the status/message area.
    • Select the added run from the Direct Beam table by clicking its radio button — confirm the lower-right plot displays "Intensity" (not "Reflectivity").
    • Try to add the same run again — confirm a blocking error message appears and no duplicate is added.
    • Load another scattering run, add it to the Data table, set its direct_beam parameter to the previously added run, and run reflectivity calculation to confirm normalization works.
    • Optional: Add the same run to both Direct Beam and Data tables — confirm that selecting it from either table shows "Intensity" plot (this is expected behavior for ROI comparison workflows).

(instructions to set up the environment)

Check list for the reviewer

  • best software practices
    • clearly named variables (better to be verbose in variable names)
    • code comments explaining the intent of code blocks
  • All the tests are passing
  • The documentation is up to date and looks correct in the pull request preview
  • code comments added when explaining intent

@codecov-commenter

codecov-commenter commented Nov 11, 2025

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 98.76543% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 65.57%. Comparing base (ad9201b) to head (7973419).
⚠️ Report is 9 commits behind head on next.

Files with missing lines Patch % Lines
...quicknxs/interfaces/event_handlers/main_handler.py 98.48% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             next     #229      +/-   ##
==========================================
- Coverage   65.72%   65.57%   -0.15%     
==========================================
  Files          37       37              
  Lines        6909     6914       +5     
==========================================
- Hits         4541     4534       -7     
- Misses       2368     2380      +12     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

This comment was marked as resolved.

Comment thread src/quicknxs/interfaces/data_manager.py Outdated
@darshdinger darshdinger closed this Jan 8, 2026
@darshdinger
darshdinger deleted the ewm11847-add-ability-to-load-non-Direct_beam-labeled-pv-data branch January 8, 2026 20:16
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.

4 participants