Conversation
Bumps [js-yaml](https://github.com/nodeca/js-yaml) from 4.1.0 to 4.1.1. - [Changelog](https://github.com/nodeca/js-yaml/blob/master/CHANGELOG.md) - [Commits](nodeca/js-yaml@4.1.0...4.1.1) --- updated-dependencies: - dependency-name: js-yaml dependency-version: 4.1.1 dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com>
…l-4.1.1 Bump js-yaml from 4.1.0 to 4.1.1
There was a problem hiding this comment.
Pull Request Overview
This PR addresses two issues: Python 3.9 compatibility and a fix for the reasoning_effort parameter with GPT-5.1. The changes lower the minimum Python version requirement from 3.12 to 3.9 and update the reasoning_effort parameter from "minimal" to "low" for reasoning-capable models.
Key Changes:
- Python version requirement lowered from >=3.12 to >=3.9
- Refactored f-string expression to be compatible with Python 3.9/3.10
- Changed
reasoning_effortparameter from "minimal" to "low" for GPT-5, O1, O3, and Claude reasoning models
Reviewed Changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| pyproject.toml | Updated minimum Python version from 3.12 to 3.9 and bumped package version to 0.5.0.2 |
| py-src/data_formulator/agents/agent_utils.py | Extracted f-string expression to a variable for Python 3.9/3.10 compatibility (these versions don't support expressions inside f-strings) |
| py-src/data_formulator/agents/client_utils.py | Changed reasoning_effort parameter from "minimal" to "low" for reasoning models (gpt-5, o1, o3, and Claude models) |
| yarn.lock | Automatic dependency lock file reordering (no functional changes to dependencies) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| table_summaries = [assemble_table_summary(input_table, i) for i, input_table in enumerate(input_tables)] | ||
|
|
||
|
|
||
| # Join with newline (extracted from f-string for Python 3.9/3.10 compatibility) |
There was a problem hiding this comment.
The comment mentions Python 3.9/3.10 compatibility, but the actual issue is specific to Python 3.9 and 3.10. Python 3.11+ supports expressions with {} inside f-string literals. Consider updating the comment to be more precise:
# Join with newline (extracted from f-string for Python 3.9/3.10 compatibility)
# Python 3.9-3.10 don't support expressions like {'\n'.join(...)} inside f-stringsThis helps future maintainers understand that this workaround can be removed when the minimum Python version is raised to 3.11+.
| # Join with newline (extracted from f-string for Python 3.9/3.10 compatibility) | |
| # Join with newline (extracted from f-string for Python 3.9/3.10 compatibility) | |
| # Python 3.9-3.10 don't support expressions like {'\n'.join(...)} inside f-strings | |
| # This workaround can be removed when minimum Python version is raised to 3.11+ |
| if self.model.startswith("gpt-5") or self.model.startswith("o1") or self.model.startswith("o3"): | ||
| completion_params["reasoning_effort"] = "minimal" | ||
| completion_params["reasoning_effort"] = "low" |
There was a problem hiding this comment.
The reasoning_effort parameter change from "minimal" to "low" may be correct for the models being targeted, but this change is applied to all reasoning models (gpt-5, o1, o3, and claude models). The PR description mentions this is a temporary fix for GPT-5.1 specifically and "should be fixed later to make it more robust."
Consider:
- Adding a comment explaining why "low" was chosen and that this is a temporary solution
- Adding validation or error handling in case the API rejects the parameter value
- Documenting the valid values for
reasoning_effortfor different model families
This would help prevent issues if different models support different reasoning_effort values.
fix issues: