Skip to content

Unit overhaul 2/6: Centralize Q-index validation via verify_Q_index - #222

Merged
henrikjacobsenfys merged 1 commit into
developfrom
split/02a-verify-q-index
Aug 9, 2026
Merged

Unit overhaul 2/6: Centralize Q-index validation via verify_Q_index#222
henrikjacobsenfys merged 1 commit into
developfrom
split/02a-verify-q-index

Conversation

@henrikjacobsenfys

@henrikjacobsenfys henrikjacobsenfys commented Jul 6, 2026

Copy link
Copy Markdown
Member

Replace the AnalysisBase._verify_Q_index method and the various inline Q_index type/bounds checks scattered across the model hierarchy with the shared verify_Q_index helper (added in the previous PR). Callers now delegate validation to a single function with consistent error messages and behavior (TypeError for non-int, IndexError for negative/out-of-range, upper-bound deferred when Q is None).

Migrated: Analysis, Analysis1d, AnalysisBase, Experiment, ModelBase, InstrumentModel, DiffusionModelBase and DeltaLorentz. Tests updated to the unified messages.

Net: 15 files, +71/−236 (removes duplicated validation blocks). All unit tests pass.


Stack — splitting the monolithic "Unit system overhaul" (#217) into a reviewable series. Merge bottom-up:

  1. Add unit-conversion utilities and FitTarget → develop (Unit overhaul 1/6: Add unit-conversion utilities and FitTarget #221)
  2. Centralize Q-index validation via verify_Q_indexsplit/01-unit-utils (Unit overhaul 2/6: Centralize Q-index validation via verify_Q_index #222) (this PR)
  3. Store Q as a scipp Variable in 1/angstrom → split/02a-verify-q-index (Unit overhaul 3/6: Store Q as a scipp Variable in 1/angstrom #223)
  4. Separate model unit into x_unit/y_unit + update tutorials → split/02b-q-variable (Unit overhaul 4/6: Separate unit into x_unit/y_unit and update tutorials #224)

🤖 Generated with Claude Code

@henrikjacobsenfys henrikjacobsenfys added [scope] enhancement Adds/improves features (major.MINOR.patch) [priority] medium Normal/default priority labels Jul 6, 2026
@codecov

codecov Bot commented Jul 6, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.01%. Comparing base (36bef4d) to head (b170048).

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #222      +/-   ##
===========================================
+ Coverage    97.92%   98.01%   +0.09%     
===========================================
  Files           53       53              
  Lines         3707     3685      -22     
  Branches       660      640      -20     
===========================================
- Hits          3630     3612      -18     
+ Misses          45       43       -2     
+ Partials        32       30       -2     
Flag Coverage Δ
unittests 98.01% <100.00%> (+0.09%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
src/easydynamics/analysis/analysis.py 95.06% <100.00%> (+0.02%) ⬆️
src/easydynamics/analysis/analysis1d.py 98.60% <100.00%> (+0.01%) ⬆️
src/easydynamics/analysis/analysis_base.py 100.00% <ø> (ø)
src/easydynamics/experiment/experiment.py 97.66% <100.00%> (ø)
...mics/sample_model/diffusion_model/delta_lorentz.py 97.57% <100.00%> (-0.01%) ⬇️
...mple_model/diffusion_model/diffusion_model_base.py 96.87% <100.00%> (+2.93%) ⬆️
src/easydynamics/sample_model/instrument_model.py 100.00% <100.00%> (ø)
src/easydynamics/sample_model/model_base.py 97.56% <100.00%> (-0.10%) ⬇️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@henrikjacobsenfys henrikjacobsenfys changed the title Unit overhaul 2/5: Centralize Q-index validation via verify_Q_index Unit overhaul 2/4: Centralize Q-index validation via verify_Q_index Jul 6, 2026
@henrikjacobsenfys henrikjacobsenfys changed the title Unit overhaul 2/4: Centralize Q-index validation via verify_Q_index Unit overhaul 2/6: Centralize Q-index validation via verify_Q_index Jul 7, 2026
@henrikjacobsenfys
henrikjacobsenfys force-pushed the split/02a-verify-q-index branch from a5ff526 to c33a479 Compare July 8, 2026 08:52
Replace the AnalysisBase._verify_Q_index method and the various inline
Q_index type/bounds checks scattered across the model hierarchy with the
shared verify_Q_index helper (added in the previous PR). Callers now
delegate validation to a single function with consistent error messages
and behavior (TypeError for non-int, IndexError for negative/out-of-range,
upper-bound deferred when Q is None).

Migrated: Analysis, Analysis1d, AnalysisBase, Experiment, ModelBase,
InstrumentModel, DiffusionModelBase and DeltaLorentz. Tests updated to the
unified messages. Split out of the larger unit-system branch.

All unit tests pass.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@henrikjacobsenfys
henrikjacobsenfys force-pushed the split/02a-verify-q-index branch from c33a479 to b170048 Compare August 9, 2026 18:51
@henrikjacobsenfys
henrikjacobsenfys changed the base branch from split/01-unit-utils to develop August 9, 2026 18:53
@henrikjacobsenfys
henrikjacobsenfys merged commit 9abb18f into develop Aug 9, 2026
20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

[priority] medium Normal/default priority [scope] enhancement Adds/improves features (major.MINOR.patch)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant