Abstract syntax tree parser for ConfigSpace#413
Conversation
|
TODO: Add more docs into the methods, s.t. we have examples for the documentation site |
|
Added new docs example for usage, cleaned up code and other comments. I think its ready for a review now. |
|
@mfeurer Reminder for this PR |
There was a problem hiding this comment.
Pull request overview
Adds an AST-based parser that converts string logic expressions into ConfigSpace Condition or ForbiddenClause objects, to support importing ConfigSpaces from external notations (e.g., PCS/SMAC/IRACE).
Changes:
- Introduces
expression_to_configspace()(and a recursive AST conversion helper) to map comparisons and boolean logic into ConfigSpace Conditions/Forbiddens. - Adds unit tests covering basic parsing cases (value comparisons, HP relations, conjunctions/disjunctions,
in). - Extends imports to include additional Forbidden clause/conjunction types used in the new tests.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 7 comments.
| File | Description |
|---|---|
src/ConfigSpace/util.py |
Adds AST parsing and recursive conversion into ConfigSpace Condition/Forbidden objects. |
test/test_util.py |
Adds tests for expression_to_configspace() covering simple/complex expressions and error cases. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
test/test_util.py
Outdated
| wrong_expression = "a >!> b" | ||
| with pytest.raises(ValueError): | ||
| expression_to_configspace(wrong_expression, cs) | ||
|
|
||
| wrong_hp_name_expresion = "q <= 5" | ||
| with pytest.raises(ValueError): | ||
| cs_expression = expression_to_configspace(wrong_hp_name_expresion, cs) | ||
|
|
||
| wrong_hp_value_expression = "a > 11" | ||
| with pytest.raises(ValueError): | ||
| cs_expression = expression_to_configspace(wrong_hp_value_expression, cs) | ||
|
|
||
| wrong_hp_value_expression = "a == dog" | ||
| with pytest.raises(ValueError): | ||
| cs_expression = expression_to_configspace(wrong_hp_value_expression, cs) |
There was a problem hiding this comment.
Did you add the check for "a != b"?
src/ConfigSpace/util.py
Outdated
| expression = re.sub(r" != ", " = ", expression) | ||
| expression = re.sub(r" (?<![<>!=])=(?<![=]) ", " == ", expression) |
There was a problem hiding this comment.
It seems like you have not incorporated this, was this wrong?
…tract-syntax-tree-parser
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
|
Hi, I'm finally able to wrap my head around what TL;DR: Summary
Suggestions
|
|
|
Fair points! I don't have strong opinions about the user base or the naming conventions yet 😄. We can merge if @mfeurer also doesn't have strong opinions against it. |
mfeurer
left a comment
There was a problem hiding this comment.
Here we go. I think the code is fine, but I have not gone through the whole logic of the parsing and rather assume that the tests are correct.
I don't think that the function name is great yet, maybe something like parse_expression_from_string(). Besides that, I don't have strong opinions on having one or two functions, and think we can go with a single function for now.
test/test_util.py
Outdated
| wrong_expression = "a >!> b" | ||
| with pytest.raises(ValueError): | ||
| expression_to_configspace(wrong_expression, cs) | ||
|
|
||
| wrong_hp_name_expresion = "q <= 5" | ||
| with pytest.raises(ValueError): | ||
| cs_expression = expression_to_configspace(wrong_hp_name_expresion, cs) | ||
|
|
||
| wrong_hp_value_expression = "a > 11" | ||
| with pytest.raises(ValueError): | ||
| cs_expression = expression_to_configspace(wrong_hp_value_expression, cs) | ||
|
|
||
| wrong_hp_value_expression = "a == dog" | ||
| with pytest.raises(ValueError): | ||
| cs_expression = expression_to_configspace(wrong_hp_value_expression, cs) |
There was a problem hiding this comment.
Did you add the check for "a != b"?
src/ConfigSpace/util.py
Outdated
| expression = re.sub(r" != ", " = ", expression) | ||
| expression = re.sub(r" (?<![<>!=])=(?<![=]) ", " == ", expression) |
There was a problem hiding this comment.
It seems like you have not incorporated this, was this wrong?
Adding in logic for parsing expressions into ConfigSpace Conditions/Forbiddens. This is aimed to take general logic expressions (strings) and uses the AST library to convert it into a ConfigSpace expression.
The reason of this being necessary is as a build up towards enabling parsing a ConfigSpace from other notation files, such as PCS files from SMAC2, IRACE etc.