Fix some issues related to different polars versions #8
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR fixes compatibility issues with different Polars versions by updating aggregation operations. The changes replace deprecated methods with more current Polars API calls to ensure compatibility across versions.
- Replaced
map_elementswith simpler mathematical operations for geometric mean calculation - Updated
applymethod call for custom operations - Modified infinity handling in running time aggregations using conditional logic instead of
replace
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| ) | ||
| if max_budget is None: | ||
| max_budget = data[evaluation_variable].max() | ||
| max_budget = data[evaluation_variable].max()+1 |
There was a problem hiding this comment.
Missing space around the '+' operator. Should be max_budget = data[evaluation_variable].max() + 1 for consistency with Python style guidelines.
| max_budget = data[evaluation_variable].max()+1 | |
| max_budget = data[evaluation_variable].max() + 1 |
| ).alias("ERT"), | ||
| ( | ||
| pl.col(evaluation_variable).replace(np.inf, max_budget * 10).sum() | ||
| pl.col(evaluation_variable).sum() + pl.col(evaluation_variable).is_between(max_budget, np.inf).count() * max_budget * 9 |
There was a problem hiding this comment.
The magic number 9 should be extracted as a named constant or variable (e.g., PAR_MULTIPLIER = 9) to improve code readability and maintainability.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 2 out of 3 changed files in this pull request and generated 2 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| pl.col(evaluation_variable) | ||
| .map_elements(lambda s: custom_op(s), return_dtype=pl.Float64) | ||
| .alias(custom_op.__name__) | ||
| pl.col(fval_variable).apply(custom_op).alias(custom_op.__name__) |
There was a problem hiding this comment.
The column reference should be evaluation_variable instead of fval_variable to match the function parameter and maintain consistency with the convergence function above.
| if max_budget is None: | ||
| max_budget = data[evaluation_variable].max() | ||
| max_budget = data[evaluation_variable].max()+1 |
There was a problem hiding this comment.
Adding 1 to max_budget without explanation creates a magic number. Consider adding a comment explaining why this adjustment is necessary or define it as a named constant.
by modifying aggregations, this should fix the per-function plots for different polars versions