Skip to content

Handle unsupported logger types in training logger#463

Open
Ritinikhil wants to merge 1 commit intomllam:mainfrom
Ritinikhil:main
Open

Handle unsupported logger types in training logger#463
Ritinikhil wants to merge 1 commit intomllam:mainfrom
Ritinikhil:main

Conversation

@Ritinikhil
Copy link
Copy Markdown
Contributor

Raise ValueError for unsupported logger types.

Describe your changes

  • Add an else branch in setup_training_logger (neural_lam/utils.py) to raise a descriptive ValueError when an unsupported --logger value is provided.
  • Ensure the function now either returns a valid logger (WandB/MLflow) or raises a clear error, preventing the previous UnboundLocalError.

Issue Link

closes #206

Type of change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✨ New feature (non-breaking change that adds functionality)
  • 💥 Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • 📖 Documentation (Addition or improvements to documentation)

Checklist before requesting a review

  • My branch is up-to-date with the target branch.
  • I have performed a self-review of my code.
  • For any new/modified functions/classes I have added docstrings that clearly describe its purpose, expected inputs and returned values. (No new docstrings required; existing docstring unchanged.)
  • I have placed in-line comments to clarify the intent of any hard-to-understand passages of my code.
  • I have updated the README to cover introduced code changes.
  • I have added tests that prove my fix is effective or that my feature works.
  • I have given the PR a name that clearly describes the change, written in imperative form.
  • I have requested a reviewer and an assignee.

Checklist for reviewers

  • the code is readable
  • the code is well tested
  • the code is documented (including return types and parameters)
  • the code is easy to maintain

Author checklist after completed review

  • I have added a line to the CHANGELOG describing this change, in a section reflecting type of change (add/changed/fixes/maintenance)

Checklist for assignee

  • PR is up to date with the base branch
  • the tests pass
  • the PR is assigned to the next milestone (if applicable)
  • author has added an entry to the changelog
  • squash commits and merge when ready

Raise ValueError for unsupported logger types.
Copilot AI review requested due to automatic review settings March 20, 2026 10:10
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Fixes a bug in setup_training_logger by ensuring unsupported --logger values fail fast with a clear ValueError instead of potentially triggering an UnboundLocalError.

Changes:

  • Add an explicit else branch in setup_training_logger to raise a descriptive ValueError for unsupported logger types.
  • Restructure the function to return the created logger directly from each supported branch.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +544 to +548
else:
raise ValueError(
f"Unsupported logger type: {args.logger!r}. "
"Supported loggers are: 'wandb', 'mlflow'."
)
Copy link

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

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

Add a unit test asserting that an unsupported args.logger value raises ValueError with a helpful message. There are already setup_training_logger tests in tests/test_cli.py, so extending coverage here will prevent regressions back to an UnboundLocalError/silent misconfiguration.

Copilot uses AI. Check for mistakes.
@sadamov sadamov added the bug Something isn't working label Mar 23, 2026
@sadamov sadamov self-requested a review March 23, 2026 05:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug Found: setup_training_logger function doesn't return a logger when an unsupported logger type is specified

3 participants