-
-
Notifications
You must be signed in to change notification settings - Fork 412
prevent adding methods to the functions > and >=
#4065
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
Conversation
As documented, the intended way to implement `>` is to add a method to `<`. Similarly with `>=`. A package should never add a method to either `>` or `>=`.
odow
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.
Assuming the fallbacks work
Correct error message for variable comparison in Julia.
Correct error messages in nonlinear.md for clarity.
| julia> f(x) | ||
| ERROR: Cannot evaluate `>` between a variable and a number. | ||
| ERROR: Cannot evaluate `<` between a variable and a number. |
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 is the only downside. We might talk about < when the user wrote >.
|
Aside from recommended practice, what is the motivation for this? |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #4065 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 43 43
Lines 6201 6201
=========================================
Hits 6201 6201 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
This PR, together with PR JuliaDiff/ForwardDiff.jl#771, greatly decreases the amount of sysimage invalidation on
EDIT: the above is with nightly (v1.13) Julia, JuliaLang/julia@865b8be |
|
Status with this PR, but without PR JuliaDiff/ForwardDiff.jl#771:
So an improvement, but not as much as with PR JuliaDiff/ForwardDiff.jl#771. |
|
NB: if you want to reproduce the above results, do |
|
Invalidation is a good reason |
As documented, the intended way to implement
>is to add a method to<. Similarly with>=. A package should never add a method to either>or>=.