Skip to content

Fix: Add algorithm name validation in OsipiBase.osipi_initiate_algorithm()#160

Merged
IvanARashid merged 2 commits into
OSIPI:mainfrom
Devguru-codes:algoname
May 5, 2026
Merged

Fix: Add algorithm name validation in OsipiBase.osipi_initiate_algorithm()#160
IvanARashid merged 2 commits into
OSIPI:mainfrom
Devguru-codes:algoname

Conversation

@Devguru-codes

@Devguru-codes Devguru-codes commented May 4, 2026

Copy link
Copy Markdown
Contributor

This PR fixes the issue where OsipiBase crashes with a confusing ModuleNotFoundError or AttributeError when initialized with a misspelled or invalid algorithm name. The dynamic class loader in osipi_initiate_algorithm() previously performed zero validation before calling importlib.import_module(), causing raw Python import errors to surface to the user.

Changes Made

src/wrappers/OsipiBase.py

  1. Algorithm name validation: Before attempting the dynamic import, the method now scans the src/standardized/ directory using pathlib.glob("*.py") to build a set of valid algorithm names at runtime. This approach is always in sync with the actual codebase — no hardcoded list needed.

  2. Clear ValueError: If the requested algorithm is not found, a ValueError is raised listing all available algorithms alphabetically, making typos easy to spot.

  3. Secondary safety net: The importlib.import_module() and getattr() calls are now wrapped in explicit try/except blocks with descriptive error messages:

    • ImportError — if the file exists but fails to import (e.g., syntax error in the module).
    • AttributeError — if the module loads but doesn't contain the expected class name.
  4. Updated docstring: Added Raises section documenting the ValueError.

tests/IVIMmodels/unit_tests/test_ivim_fit.py

Added test_invalid_algorithm_name() regression test with 3 sub-checks:

  1. A typo ("IAR_LU_biepx") raises ValueError with "not found" in the message.
  2. A completely fake name ("totally_fake_algorithm") raises ValueError that includes "Available algorithms are:" and lists known algorithms like IAR_LU_biexp and PV_MUMC_biexp.
  3. A valid algorithm ("IAR_LU_biexp") still initializes successfully without errors (no false positives).

Testing & Validation

$ python -m pytest tests/IVIMmodels/unit_tests/test_ivim_fit.py::test_invalid_algorithm_name -v
tests/.../test_ivim_fit.py::test_invalid_algorithm_name PASSED [100%]
1 passed in 1.81s

$ python -m pytest tests/IVIMmodels/unit_tests/test_ivim_fit.py::test_default_bounds_and_initial_guesses -v
22 passed, 5 skipped in 2.24s
  • New test passes — invalid names correctly raise ValueError
  • Existing tests unaffected — all 22 algorithm-loading tests still pass (5 MATLAB/DL skipped as expected)

fixes #159

…thm() - Validates algorithm name against src/standardized/ before dynamic import - Raises clear ValueError listing available algorithms for typos - Adds try/except safety net for ImportError and AttributeError - Adds test_invalid_algorithm_name regression test

@IvanARashid IvanARashid left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, just one more adjustment so that we stay consistent.

Comment thread src/wrappers/OsipiBase.py Outdated
@Devguru-codes

Copy link
Copy Markdown
Contributor Author

Looks good, just one more adjustment so that we stay consistent.

I have implemented the change sir. Thank you.

@oliverchampion

Copy link
Copy Markdown
Collaborator

Looks good to me. If Ivan agrees we can merge

@IvanARashid IvanARashid left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good now, thanks!

@IvanARashid IvanARashid merged commit 906d86e into OSIPI:main May 5, 2026
9 checks passed
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.

[BUG] No error handling or validation in OsipiBase.__init__() for invalid algorithm names

3 participants