Skip to content

Fix VFA validation for NumPy flip angles - #162

Closed
ParikshitGupta2605 wants to merge 4 commits into
OSIPI:mainfrom
ParikshitGupta2605:fix-vfa-numpy-flip-angles
Closed

Fix VFA validation for NumPy flip angles#162
ParikshitGupta2605 wants to merge 4 commits into
OSIPI:mainfrom
ParikshitGupta2605:fix-vfa-numpy-flip-angles

Conversation

@ParikshitGupta2605

Copy link
Copy Markdown
Contributor

Summary

This PR fixes the validation of flip_angles in compute_t1_vfa() when provided as a NumPy array.

Previously, the code used:

if not params.flip_angles:

which raises a ValueError for NumPy arrays because their truth value is ambiguous.

Changes

  • Replace the boolean truth-value check with explicit validation for None and empty arrays.
  • Convert flip_angles using np.asarray() before validation.
  • Add a regression test to ensure NumPy arrays are accepted.

Fixes #159

@ltorres6

Copy link
Copy Markdown
Collaborator

@MohamedNasser8 can you review this one when you have some time?

@MohamedNasser8

Copy link
Copy Markdown
Collaborator

Of course

@MohamedNasser8

Copy link
Copy Markdown
Collaborator

Thanks @ParikshitGupta2605

My one concern is where to fix it. This solves it inside vfa.py, but flip_angles actually belongs to DCEAcquisitionParams, and other parts of the code read it too (e.g. signal-to-concentration) . The field is declared as list[float] but nothing actually enforces that. That means every other function depends on DCEAcquisitionParams keeps the same bug.

I think it's better to fix it once at the dataclass for example use post_init to keep it list. I mean convert whatever is passed into a plain list of floats, and raise a clear error if it genuinely can't be converted.

@ParikshitGupta2605

Copy link
Copy Markdown
Contributor Author

Thanks for the feedback! Normalizing flip_angles in DCEAcquisitionParams.post_init is a better approach since it guarantees a consistent list[float] representation for all consumers, rather than handling it only in vfa.py. I'll update the implementation accordingly and simplify the VFA-specific changes.

@ltorres6

Copy link
Copy Markdown
Collaborator

Thanks both. Also note there are several locations where we cast as list or as array that probably needs to be cleaned up once this is implemented in the dataclass.

@ParikshitGupta2605
ParikshitGupta2605 deleted the fix-vfa-numpy-flip-angles branch July 29, 2026 14:00
@ParikshitGupta2605
ParikshitGupta2605 restored the fix-vfa-numpy-flip-angles branch July 29, 2026 14:00
@ParikshitGupta2605
ParikshitGupta2605 deleted the fix-vfa-numpy-flip-angles branch July 29, 2026 14:00
@ParikshitGupta2605
ParikshitGupta2605 restored the fix-vfa-numpy-flip-angles branch July 29, 2026 14:00
@ParikshitGupta2605

Copy link
Copy Markdown
Contributor Author

Reopening with the final version - Pull request #167

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.

vfa.py bug

3 participants