Deprecate fzero - #523
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR deprecates legacy root-finding interfaces while retaining compatibility through find_zero and find_zeros.
Changes:
- Adds deprecation warnings for
fzero,fzeros, and related interfaces. - Relocates deprecated source and tests.
- Updates module inclusion and test execution order.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Review |
|---|---|
test/test_fzero.jl |
Reviewed; no final issues. |
test/runtests.jl |
Reviewed; no final issues. |
src/Roots.jl |
Reviewed; no final issues. |
src/deprecated/alternative_interfaces.jl |
Moderate issue (4 votes): migration hints use nonexistent Roots.Quadratic_Inverse() instead of Roots.QuadraticInverse() at lines 86, 250, and 332. |
Suppressed comments (6)
src/deprecated/alternative_interfaces.jl:86
- This migration hint names
Roots.Quadratic_Inverse(), but that constructor does not exist; the method isRoots.QuadraticInverse(). Following this deprecation warning therefore leads users to anUndefVarErrorinstead of the replacement API.
This issue also appears in the following locations of the same file:
- line 250
- line 332
src/deprecated/alternative_interfaces.jl:332
Base.depwarnuses its symbol argument to identify the deprecated feature, butfzerosis registered under:fzero. This conflates the two deprecations (including warning suppression/deduplication) and should use thefzeroskey.
src/deprecated/alternative_interfaces.jl:171- This overload also accepts the documented
orderkeyword, but the hint drops it: for example,fzero(sin, 3; order=16)usesOrder16()while the suggestedfind_zero(sin, 3)silently uses the defaultOrder0(). The deprecation guidance should translate/preserve the selected method (and the legacy tolerance aliases) rather than claim a universal direct replacement.
src/deprecated/alternative_interfaces.jl:218 - When
orderis supplied, this overload selects that method via_method_lookup, but the hint removes it and changes the call tofind_zero(f, (a,b)), whose default isBisection(). For example, anorder=1call would no longer use the requested secant method; the migration text should preserve the selected method.
src/deprecated/alternative_interfaces.jl:250 - The deprecated overload explicitly invokes
Newton()below, but this hint omits the method.find_zero((f, fp), x0)defaults toOrder0()for a scalarx0, whose single-output wrapper takes only the first tuple component, so following the hint silently switches to derivative-free solving and ignoresfp. IncludeRoots.Newton()in the replacement.
test/runtests.jl:44 - These tests exercise the deprecated calls but never assert that the new warnings are emitted, so the PR's primary behavior can regress while the suite remains green. Add
@test_deprecatedchecks for representativefzero/fzerosoverloads and the derivative-interface functions instead of only relocating the existing functional tests.
# deprecated
include("./test_fzero.jl")
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #523 +/- ##
==========================================
- Coverage 86.94% 86.80% -0.15%
==========================================
Files 42 43 +1
Lines 2628 2697 +69
==========================================
+ Hits 2285 2341 +56
- Misses 343 356 +13 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add deprecation warning to fzero, fzeros and some other functions. Decide another day if those functions in simple should be deprecated. This will replace #507