-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
Julia 0.5 now complains about redefining methods #18725
Comments
... at least it should be possible to configure not to display the warning, if the new method replaces the old method in the same position "WARNING: Method definition saveplot(Any) in module Main at /home/daniel/Science/C4/misc.jl:6 |
I think it's reasonable to allow these warnings to be turned off. It might be a good idea to turn them off by default, although I like them quite a lot for their linter-like quality. @dcarrera: I think you might want to note that the languages you refer to for precedents are all interpreted languages, but Julia is in some ways closer to a compiled language like C, where you recompile your code every time you change something. |
@johnmyleswhite "Julia is in some ways closer to a compiled language like C, where you recompile your code" is somehow well understood, but julia tries in other places hard to hide this from the user. The 99% case of julia usage is the REPL. |
To note is that overwriting functions creates the possibility of silently getting the wrong answer. JuliaPlots/Plots.jl#508 |
Perhaps we should note that making it safe to overwrite functions probably requires waiting for #265 to be resolved? |
@KristofferC Unfortunately overwriting functions creates the possibility of getting the job done without constantly restarting julia. |
@lobingera You can always run |
@mbaz this may not be a solution if one needs to retain other defined variables. |
@johnmyleswhite The languages I list as precedent are not interpreted languages; they compile to a virtual machine. I realize that it's still not the same thing as Julia, but just saying... |
I can see the importance for debugging a work in progress but I think the current reaction shouldn't be default. Keywords instead?
Even some sort of broader setting like Please do make this setting off by default. |
Other people have said a lot about this annoying feature. One thing I want to add is that no one would find this feature useful when he is repeatedly bombarded with hundreds of the warning messages flashing by the terminal. No Julia programmer is a robot at the moment. |
putting your code in a module might cut down on the warnings? this warning identified at least a dozen or more bugs in base and package code. it could be possibly disabled for interactive repl usage, but in library code method overwrites are usually a sign of a bug or attempt at monkey patching. |
I have made this package that I've been using called Suppressor: julia> using Suppressor
julia> @suppress begin
println("This string doesn't get printed!")
warn("This warning is ignored.")
end
julia> @suppress_out begin
println("This string doesn't get printed!")
warn("This warning is important")
end
WARNING: This warning is important
julia> @suppress_err begin
println("This string gets printed!")
warn("This warning is unimportant")
end
This string gets printed!
julia> @suppress begin
println("This string doesn't get printed!")
warn("This warning is ignored.")
error("Remember that errors are still printed!")
end
------------------------------------------------------------------------------------------
ErrorException Stacktrace (most recent call last)
[#2] — anonymous
⌙ at <missing>:?
[#1] — macro expansion;
⌙ at Suppressor.jl:16 [inlined]
Remember that errors are still printed!
julia> On a different note, why does STDERR does not redirect errors!? This is the definition of macro suppress_err(block)
quote
if ccall(:jl_generating_output, Cint, ()) == 0
ORIGINAL_STDERR = STDERR
err_rd, err_wr = redirect_stderr()
err_reader = @async @compat readstring(err_rd)
value = $(esc(block))
@async wait(err_reader)
REDIRECTED_STDERR = STDERR
err_stream = redirect_stderr(ORIGINAL_STDERR)
return value
end
end
end How about this macros (better versions of them due to reviews of course) or something similar, where in Base? I made them out of the need to suppress warnings that the end user should not worry aobut, in my ongoing quest to get how IO works in Julia, what I basically ended up doing was searching for different people implementing the same functionality individually over and over again, some worked, some didn't, some didn't work on windows, etc. After looking to several of this implementations I played with all of them until I got this macros (which I do not entirely understand!), that is I tried to add the ideas of all the different implementations I found and tested, and tested until eventually it worked AFAIKT. Having people figure out this by themselves individually is ridiculous, so I ended up putting it in a package, that I've been testing ever since. Here are some examples of use cases that I'm aware of:
I wouldn't like this warnings to be turned of though, I'll adhere to the python philosophy here of explicit is better than implicit, we just need a standard and well tested way of explicitly stating that we do not want warnings or output for the following expression or block of code programmatically. Finally to answer to @dcarrera you could solve it like this: julia> @suppress_err include("foo.jl"); In the above example, STDOUT output is still emitted, this does not hide errors as currently implemented, it would only hide the warnings. |
@Ismael-VC Hello. I do appreciate your effort, but |
https://github.com/cstjean/ClobberingReload.jl#silencing-warnings perhaps
|
+1 to not emitting this warning when |
When is that thing false? |
Importantly, both of those statements are true at the REPL. They are not true for library code: the first is true for |
-1 for making warning defaults depend on whether precompilation is enabled. Behaving differently for Main might be justified though, since almost all library code should have a separate parent module. |
@tkelman fwiw, we already have a number of other errors that are only enabled for |
These warnings are annoying and are affecting most of us. It would be great to have this gone seamlessly without hacks from the user side. |
Method overwriting isn't specific to precompilation though. I would hope the others are. |
What is the danger that this warning is trying to warn against? @JeffBezanson since, IIRC, you added this when you did the 0.5 function revamp. |
Just as a counterargument to this, Julia functions are constants:
And function overwriting has nothing to do with the function's variable, it's an alteration of its structure. And, lemmytellya, we really shouldn't remove these warnings until #265 is sorted out, you don't know how many headaches I had in 0.3-0.4 until I decided to put my test scripts into modules or functions, and these just remind me to do that. It's two lines, c'mon. With modules you don't even need to reindent to stay within the coding guidelines ;P Or just add something like this to your .juliarc: function include_test(filename)
m = Module()
eval(m, :($include($filename)))
m
end julia> T = include_test("mahfile.jl")
anonymous
julia> T.f(10)
12 |
Perhaps the messages could be conditional on an environment variable, so all users can get what they want and change their minds in mid-session? char *env = getenv("JULIA_QUIET_OVERWRITE")
if (env && strcmp(env, "0") != 0) {
write_irritating_messages();
} |
As long as we have a I am not going to go on about this, but nobody mentioned this directly and it's really important: julia> f(x) = 1
f (generic function with 1 method)
julia> map(f, [0])
1-element Array{Int64,1}:
1
julia> f(x) = 2
WARNING: Method definition f(Any) in module Main at REPL[1]:1 overwritten at REPL[3]:1.
f (generic function with 1 method)
julia> map(f, [0])
1-element Array{Int64,1}:
1 In 0.5, because of function specialization, no longer does redefining methods break only your own functions, it breaks other's as well, including base's. (@StefanKarpinski, this could be one of the main reasons why the warnings were added in this version) Well, I am going, good luck... |
Touché. Now I see the light, and repudiate my suggestion. Modules for the win. |
@fcard WTF? Why does it do that? Redefining functions should work. Did it always do that? I don't have Julia 0.4 anymore so I cannot test; I just know that I'm used to redefining functions constantly (via include) as part of my daily workflow. But from your example it seems to me that function assignment in Julia is seriously broken, and someone added a warning instead of fixing it. Here is my own example, similar to yours: julia> foo(x) = 2x
foo (generic function with 1 method)
julia> foo(3)
6
julia> map(foo, [1,2,3])
3-element Array{Int64,1}:
2
4
6
julia> foo(x) = 3x
WARNING: Method definition foo(Any) in module Main at REPL[1]:1 overwritten at REPL[4]:1.
foo (generic function with 1 method)
julia> foo(3)
9
julia> map(foo, [1,2,3])
3-element Array{Int64,1}:
2
4
6 I see this as a failure to redefine |
@Sacha0 Thanks. Hmm... this issue has been discussed since 2011. I feel like Alice going down the rabbit hole. |
There was a work-around posted in #18353 |
@StefanKarpinski This is basically a linter warning, since it impacts the ability to get 100% code coverage, and in library code it is rarely intentional to define the same function twice, but is a common-enough mistake when refactoring or auto-generating code. The warning was always there, but was previously not always printed. The change was made to make it more predictable (and thus more useful) than the previous heuristics. @dcarrera Please keep the discussion civil. The decision to make it more predictable was made relatively independently from the compilers improved ability to optimize first-class functions. The #265 issue has been slowly becoming more significant as inference has been improving, but that doesn't mean we've been simply ignoring it. I've been progressively working towards correcting this for most of the year. While the fix wasn't ready in time for the v0.5 release, I fully expect it will be in the v0.6 release. Regardless, not until bugs like this are fixed will v1.0 be released. |
We always had this warning for methods overwritten in a different module, since that often indicated one package unintentionally (or ill-advisedly) overwriting a method defined by another. However, we found that accidental method overwrites also happen within modules. For example, two type signatures can be equal in terms of dispatch even if they do not look identical. On top of that, optimizations for higher-order functions made #265 worse, as has been observed in this thread, making it even more important to call attention to method overwrites. As @fcard said, method definitions modify functions, quite different from changing the value of a variable. I felt it was much better to have a warning while the fix is pending. The best approach here is probably to add a |
@vtjnash My apologies. I did not mean to be uncivil. I did not mean to say that you were ignoring #265 and I did not think that you were. My thinking at the time that I wrote is that if the issue has been around since 2011, it is probably complex enough that there wouldn't be a fix soon. I saw later that there was some discussion toward the end about a possible fix in v0.6 which is great. I am glad to hear that v1.0 won't be released until issues like this have been resolved. I do have a question about the fix: Does the fix mean that in the future (v0.6 or later) I will be able to redefine functions (at least in the REPL) without getting warnings or behaving in the current weird way? |
@JeffBezanson In addition to |
I am still "eh" about a flag since it's possible to prevent the problem instead of pretending it doesn't exist. For example, I've tested that using I know it's a bit annoying, but I believe putting scripts in modules is a good practice (you'll also probably want to do that if you put any const or type definition in the script), and I feel it's better to create tools to abstract that away if it's really that much of a hassle, than to add things to encourage the user not to bother. |
On the one hand, my REPL and notebooks have turned into seemingly unending streams of "WARNING:". On the other hand, though, I have shot myself in the foot a couple times already redefining a function without realizing it. Especially when I start getting lax about specifying the types of my arguments. My problem with the status quo isn't that the warnings aren't helpful. It's specifically that the amount of information printed is not interpretable by a human being. I wouldn't mind a gentle reminder "btw, you might have clobbered something you didn't mean to". What about a compromise solution? For example, when in an interactive environment,
I haven't been playing with Julia long enough to suggest an implementation, but I've done something similar before in Python. It takes a little investment up front to code (e.g., in sorting and combining the different warning types), but will be a nice feature to maintain the user-friendliness of Julia while also maintaining rigorousness. |
While this would not be my solution* : One thing to consider is Chrome DevTools approach to message spew: It does a nice job of suppressing redundant messages into the console, including a very nice use of badges. How that would work in "julia interactive" which currently includes the console REPL, IJulia, and Juno amongst others is probably complicated and would be totally different than what's possible with Chrome's UI, but it does seem likely that a few basic heuristics (something along the lines of "exponential backoff") could create a behavior that instead of producing a torrent of messages, produces at most a handful for most interactive flows, and yet when it matters, is able to save us from our own silly mistakes. Best of worlds... it seems like it should be possible if people decide that's what they want. *(Full disclosure: I'm in the camp of always letting the developer shoot themselves in the foot, and only ever introducing warnings with corresponding disable pragmas, but I also think this current behavior for this specific issue is fine till #265 is resolved) |
* add test_warn macro to Base.Test for verifying warning messages * use test_warn in compilation tests * add test for #18725 * doc links for test_warn * compile test no longer needs to save STDERR * rewrite pkg tests in terms of test_nowarn
I had the same issue when reloading a module that called the |
I recently upgraded to Julia 0.5 and now Julia raises a warning every time I redefine a function, which in my workflow happens all the time. I spent most of my day editing code and experimenting. My usual workflow looks like this:
Needless to say, every single time I
include("foo.jl")
I define all the methods infoo.jl
. Now, with the new Julia 0.5 my output looks like this:Now all the legitimate warnings from my code are buried in a sea of useless warnings telling me that I overwrote a function again. Not only is this irritating, but I also think that it makes no sense in a functional language like Julia, where functions are just variables. Julia doesn't warn me every time that I change the value of a variable, right? Because that would be silly and annoying. But Julia functions are variables and if I define them in the form of a variable assignment, like
myadd = x,y -> x+y
Julia doesn't complain. So Julia is also being inconsistent in that some function redefinitions raise errors but others don't.There is also no precedent that I know of for this kind of behaviour. In Python, when you modify a file and "import" the file again, you don't get a sea of warnings for every single function defined in that file. Ditto for Perl and the other high level languages that I can think of.
I would like to request that the old behaviour be restored. The new behaviour doesn't add anything of value and it creates a lot of hassle and frustration. Failing at that, I would like to at least have a setting that I can put my ~/.juliarc.jl file to turn off all these warnings.
The text was updated successfully, but these errors were encountered: