-
Notifications
You must be signed in to change notification settings - Fork 2
Bug 206 #208
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Bug 206 #208
Conversation
- Added checks for missing performance data in generate_report.py. - Updated ConfigurationOutput class to track performance data availability and reasons. - Improved error handling in PerformanceDataFrame for missing measurements. - Introduced parsing for unquoted IRACE categorical values in PCSConverter. - Added tests for unquoted categorical values and enhanced test coverage.
…elector metadata handling
|
Update the changeling with a new line describing the bug you've fixed |
thijssnelleman
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall looks good, we should discuss a few changes in the office
src/sparkle/CLI/generate_report.py
Outdated
| ) | ||
| report.append(table_status_values) | ||
|
|
||
| if scenario_output.performance_data_missing: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should never occur, in what situation does this happen?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, instead of creating a new property on the scenario, you can just do scenario_output.performance_data.exists() (just calling .exists on the path will give you this information directly?)
src/sparkle/tools/parameters.py
Outdated
| ) | ||
|
|
||
| @staticmethod | ||
| def parse_irace_categorical_values(raw_values: str) -> list[str]: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This makes sense, but shouldn't this be defined within the IRACE parse function? Also can we use regex instead? I know this wasn't done a lot yet in this file but regex are overall faster, less error prone and easier to read
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right! I'll also switch to regex.
…issing logic, bring settings_object to base version
…_selection.sh and streamline categorical value parsing in PCSConverter
…o configuration-space parsing and performance data cleaning
Adjusting latex.py to eliminate SciPy warnings/errors and improving data handling
Adding parser to irace for parsing categorical values in tools/parameters.py
Adding tests for configspace.py
Handling only NaN values containing performance dataframe case