Skip to content
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

Overwritten methods are not added/removed correctly #247

Closed
mboratko opened this issue Mar 6, 2019 · 4 comments
Closed

Overwritten methods are not added/removed correctly #247

mboratko opened this issue Mar 6, 2019 · 4 comments

Comments

@mboratko
Copy link

mboratko commented Mar 6, 2019

To reproduce:

julia> using Revise
julia> using Example

# edit Example.jl to add f()=2 on line 18

julia> Example.f()
2

# edit Example.jl to add f()=1 on line 17

julia> Example.f()
1    #<-- this should have still been 2

# remove the f()=1 from Example.jl

julia> Example.f()
1    #<-- now this really should be 2, since f()=2 is the only definition that exists

I have noticed other inconsistent results when conflicting methods are declared. Obviously this doesn't normally happen in the same file, most often I've seen it happen across files. In that case, deleting the function from one file makes it get removed from the session, even if it is still present in other files.

For example, if I have already added a line include("foo.jl") to Example.jl on line 18, and foo.jl is a blank file:

julia> using Revise
julia> using Example

# edit foo.jl to add f()=2 on line 1

julia> Example.f()
2

# edit Example.jl to add f()=1 on line 17 (just before the include expression)

julia> Example.f()
1    #<-- this should have still been 2

# remove the f()=1 from Example.jl

julia> Example.f()
ERROR: MethodError: no method matching f()
Stacktrace:
 [1] top-level scope at none:0

This is with Julia 1.0.3.1.

Not sure if it is relevant, but I'm also getting an error when testing Revise:

Skipping Base.active_repl
Skipping Base.active_repl_backend
Test Failed at /home/mboratko/.juliapro/JuliaPro_v1.0.3.1/packages/Revise/yp5KG/test/runtests.jl:4
  Expression: isempty(detect_ambiguities(Revise, Base, Core))
ERROR: LoadError: There was an error during testing
in expression starting at /home/mboratko/.juliapro/JuliaPro_v1.0.3.1/packages/Revise/yp5KG/test/runtests.jl:4
ERROR: Package Revise errored during testing
@mboratko mboratko changed the title Multiple functions do not get removed reliably Overwritten methods do not get removed reliably Mar 6, 2019
@mboratko mboratko changed the title Overwritten methods do not get removed reliably Overwritten methods are not added/removed correctly Mar 6, 2019
@timholy
Copy link
Owner

timholy commented Mar 6, 2019

Revise basically assumes a unique definition for each method. Julia itself complains if you violate this:

julia> push!(LOAD_PATH, "/tmp")
4-element Array{String,1}:
 "@"      
 "@v#.#"  
 "@stdlib"
 "/tmp"   

julia> using UDef
[ Info: Precompiling UDef [top-level]
WARNING: Method definition f() in module UDef at /tmp/UDef.jl:2 overwritten at /tmp/UDef.jl:3.

where /tmp/UDef.jl is defined as

module UDef
f() = 1
f() = 2
end

Moreover, if you have the same method (with the same signature) defined differently in different files, then the behavior of that function depends on the order in which you load the files---the last one loaded wins. That would mean that using A, B would give a different result from using B, A.

Is this something that's important to you? Fixing this might eventually be possible after #243, but it's basically impossible to fix with the current design.

Not sure if it is relevant, but I'm also getting an error when testing Revise:

That test is checking that there are no ambiguous methods. They pass if you run revise in a fresh REPL session with no other packages. Are you loading other packages? For example in a startup.jl file? You can try with julia --startup-file=no.

@mboratko
Copy link
Author

mboratko commented Mar 15, 2019

First off, let me say thanks for creating Revise! It is fantastic and a huge leap toward the sort of interactive-coding experience I think is most fluid and productive.

Regarding the unique definition situation, my preferred solution is guided by my current development workflow, which I am absolutely willing to adapt if you have a better suggestions. Generally I will start up a REPL and load some module, try out some functionality, make some changes in the module and then return to the REPL to try them out. In a perfect world, the state of the REPL would completely match the state of a new session which was run with the same commands, that is to say if I run

julia> using Revise
julia> using Example
julia> <some expressions A>
# make some changes to Example
julia> <some expressions B>

then if I close out of the REPL and open it up again and run the following:

julia> using Example
julia> <some expressions A>
julia> <some expressions B>

(where these expressions A and B are the same as the first REPL) then I have exactly the same state at the end of both of these programs.

Obviously (without having Revise simply keep track of the commands and restart the REPL, which defeats the point) this isn't possible, however most of the typical caveats are clear. For instance, if I am testing a function f and I write a=f() and then edit f I am aware that the value of a would be different if I rerun the program. This particular multiple-method issue tripped me up, however, because it can change the value you receive from a=f() even if you had not previously called it.

From a practical standpoint, I believe the issue arose when I was editing a function and, during one of the intermittent saves, had accidentally saved a method with the same level of type specificity as another. Fixing the issue, however, deleted the function. For example:

julia> using Revise
julia> using Example

Now, without running anything in the REPL, I edit Example to have two methods:

f(a) = "Generic method"
f(a) = "This one should apply only to Int" # but I forgot to add the type

I save the file and go back to the REPL, only to realize my mistake. Still, I haven't actually run anything in the REPL, so I go back and edit it:

f(a) = "Generic method"
f(a::Int) = "This one should apply only to Int"

Ah, that's better - let's try it out:

julia> Example.f("test")
ERROR: MethodError: no method matching f(::String)
Closest candidates are:
  f(::Int64) at /home/mboratko/.julia/dev/Example/src/Example.jl:19
Stacktrace:
 [1] top-level scope at none:0

In the original instance it wasn't this simple. The generic function f was actually defined somewhere deeper in the project and I was attempting to make a more specific version of it but during one of my edits I had left off some of the type info.


Regarding the testing - I do see the tests pass if I use vanilla Julia 1.0.3, but for some reason they do not pass with the version of Julia 1.0.3 which comes with Julia Pro. Even with julia --startup-file=no I get the following:

Skipping Base.active_repl
Skipping Base.active_repl_backend
Test Failed at /home/mboratko/.julia/packages/Revise/RiaYX/test/runtests.jl:4
  Expression: isempty(detect_ambiguities(Revise, Base, Core))
ERROR: LoadError: There was an error during testing
in expression starting at /home/mboratko/.julia/packages/Revise/RiaYX/test/runtests.jl:4
ERROR: Package Revise errored during testing

@timholy
Copy link
Owner

timholy commented Mar 21, 2019

I'm not quite sure what you mean by "go back to the REPL"---were there simply two saves of the file? Until you execute a command Revise won't perform updates (it will queue the file for processing, but that's it).

At any rate, I was not able to reproduce this. BUT I am using Revise 2.0.0, which got released yesterday. Perhaps update and see if you still have problems?

@timholy
Copy link
Owner

timholy commented Apr 6, 2019

Even with julia --startup-file=no I get the following:

That suggests you are loading some additional packages. I get this, for example, if I try running tests in Juno, but it's because of an ambiguity in Lazy.jl. Not Revise's problem. (EDIT: MikeInnes/Lazy.jl#106)

Since I can't replicate this and I think it's as fixed as it can be, I will close. Feel free to reopen if you have a clear reproducer.

@timholy timholy closed this as completed Apr 6, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants