Add descriptive ValueError when no operators are added to ComponentGenerator#91
Merged
gbomarito merged 2 commits intoJun 23, 2026
Conversation
…nerator Fixes nasa#90. When ComponentGenerator is used without calling add_operator(), random_operator() raises a cryptic IndexError from deep in ProbabilityMassFunction.draw_sample(). This adds a guard in random_operator() that raises a descriptive ValueError instead, pointing the user to call add_operator() before generating graphs. Since random_operator() is the single choke point for all operator- drawing paths (direct calls, random_operator_command(), and random_command() via PMF), the guard catches all entry paths with one check.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR improves the usability of AGraph expression generation by replacing a cryptic IndexError (triggered when no operators are configured) with a clear, actionable ValueError from ComponentGenerator.random_operator(), and adds regression tests for the affected call paths.
Changes:
- Add an explicit guard in
ComponentGenerator.random_operator()that raises a descriptiveValueErrorwhen no operators have been added. - Extend the
random_operator()docstring with aRaisessection documenting the new behavior. - Add unit tests covering direct operator selection, operator-command generation, and end-to-end generator invocation when no operators are configured.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
bingo/expressions/agraph/component_generator.py |
Adds the empty-operator guard and documents the new exception behavior. |
tests/unit/expressions/agraph/test_component_generator.py |
Adds regression tests ensuring all entry paths raise the descriptive ValueError when no operators are configured. |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Collaborator
|
Thanks for the work! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #90.
Problem
When
ComponentGeneratoris used without callingadd_operator(),random_operator()raises a crypticIndexError: list index out of rangefrom deep inProbabilityMassFunction.draw_sample(). The error gives users no indication of what went wrong or how to fix it.Fix
Add a guard in
random_operator()that checks if_operator_pmf.itemsis empty and raises a descriptiveValueErrorinstead:Since
random_operator()is the single choke point for all operator-drawing paths (direct calls,random_operator_command(), andrandom_command()→ PMF →random_operator_command()), the guard catches all entry paths with one check.Tests
Added
TestNoOperatorsErrorclass with three tests:test_random_operator_raises_without_operators— direct call torandom_operator()test_random_operator_command_raises_without_operators— viarandom_operator_command()test_generator_call_raises_without_operators— end-to-end replication viaAGraphGenerator.__call__All 31 tests in
test_component_generator.pypass. The 2 pre-existing failures intest_expression.py(sklearn__sklearn_tags__compatibility) are unrelated to this change.