Include scipy-stubs in the type-check dependencies#6174
Include scipy-stubs in the type-check dependencies#6174not522 merged 10 commits intooptuna:masterfrom
scipy-stubs in the type-check dependencies#6174Conversation
|
I'll take a look at the mypy errors |
…timize.fmin_l_bfgs_b`
…tubs` annotations
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #6174 +/- ##
==========================================
- Coverage 88.40% 88.38% -0.03%
==========================================
Files 207 207
Lines 14029 14030 +1
==========================================
- Hits 12402 12400 -2
- Misses 1627 1630 +3 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Pull Request Overview
This PR adds the "scipy-stubs" package to the type-check dependencies and makes necessary adjustments in the code to properly handle type-checking issues related to SciPy stubs. Key changes include:
- Updating test files to include detailed comments and type ignores referencing known scipy-stubs issues.
- Adding "scipy-stubs" to pyproject.toml with a Python version marker.
- Modifying functions in pruner and GP modules to improve type safety and compatibility with the new stubs.
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| tests/samplers_tests/tpe_tests/test_truncnorm.py | Added inline comments and type ignores for SciPy stubs compatibility. |
| pyproject.toml | Added "scipy-stubs" as a dependency for Python 3.10 and above. |
| optuna/pruners/_wilcoxon.py | Introduced a Literal type annotation and explicit bool conversion to safely handle np.bool_ values. |
| optuna/_gp/search_space.py | Updated Sobol engine instantiation to include a type ignore comment referencing a related SciPy stubs issue. |
| optuna/_gp/optim_mixed.py | Added a type ignore comment on function parameter to address type-check issues per a reported SciPy stubs issue. |
Comments suppressed due to low confidence (3)
optuna/_gp/search_space.py:106
- [nitpick] Consider expanding the comment to briefly explain the underlying issue referenced in scipy/scipy-stubs#644 to aid future maintainers.
qmc_engine = qmc.Sobol( # type: ignore[call-arg]
optuna/_gp/optim_mixed.py:64
- [nitpick] It might be helpful to add a concise note explaining why a type ignore is needed here (referencing scipy/scipy-stubs#645) to assist future maintainers.
func=negative_acqf_with_grad, # type: ignore[arg-type]
optuna/pruners/_wilcoxon.py:225
- Explicitly converting np.bool_ to bool is good practice; ensure that similar conversions are applied consistently across the codebase for uniform behavior.
return bool(p < self._p_threshold)
|
To avoid a large conflict, let me check this PR again after the following PR is merged: Cc: > @not522 |
I don't think there's anything there that would cause merge conflicts here. |
not522
left a comment
There was a problem hiding this comment.
Thank you for your PR! Since scipy-stubs has been updated, please make corresponding updates. 🙏
Co-authored-by: Naoto Mizuno <gobou522@gmail.com>
| a_arr, b_arr = np.array([a]), np.array([b]) | ||
| assert truncnorm_ours._log_gauss_mass(a_arr, b_arr) == pytest.approx( | ||
| _log_gauss_mass_scipy(a_arr, b_arr), nan_ok=True | ||
| ), f"_log_gauss_mass(a={a}, b={b})" |
There was a problem hiding this comment.
I noticed that the x was unused, so I removed the loop; hope you don't mind.
There was a problem hiding this comment.
While strictly speaking this change should be in a separate PR, but I think it's acceptable to do it in this PR.
I noticed some other stubs-only packages, so I figured you might also be interested in
scipy-stubs.And in case it matters (for CI runtime perhaps),
scipydoesn't explicitly requirescipy. It's also very lightweight, and it requires no mypy plugins, imports, or configuration.