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

creload does not follow include correctly in Juno #3

Closed
TsurHerman opened this issue Sep 29, 2016 · 13 comments
Closed

creload does not follow include correctly in Juno #3

TsurHerman opened this issue Sep 29, 2016 · 13 comments

Comments

@TsurHerman
Copy link

If my module is in a different directory , and it includes several files

i.e

module A
include("1.jl")
include("2.jl")
end

then creload fails to parse the file correctly.

As a workaround I use full paths. Thanks again for your work

@cstjean
Copy link
Owner

cstjean commented Sep 29, 2016

Thank you for the report! I haven't been able to reproduce the problem so far. What is the error message? Are 1.jl and 2.jl in the same directory as the A module file? Do you manually push the module's directory onto LOAD_PATH, or is it in your .julia/v0.5 directory?

On my system, running creload("ClobberingReload") from any directory works, and that's a module that includes two files.

@TsurHerman
Copy link
Author

Yes I manually add the module path to LOAD_PATH . In .julia/v0.5 I have
only code that is not mine .

I am working on a project(AR glasses) I have more than one conceptual
package - HW rendering tracking etc

So it is not logical for me to work as a package developer for Julia would
work.

Anyway .. I was looking through your code and I think that if you include
macro exapansion (macroexpand() )
In the code that parses all expressions . It can maybe help overcome some
limitations still existent in your clobber method.
So the idea is to parse expressions .. If expression is a macro then
macroexpand it and include the result which is also an expression instead
of the original expression

I tried to meddle with your code but I am not that proficient and my
schedule now forces me to settle for solutions that work now even if they
are not the most general solutions . I believe you understand this.

:-)

Alright then , enjoy the weekend.

On Friday, 30 September 2016, Cédric St-Jean notifications@github.com
wrote:

Thank you for the report! I haven't been able to reproduce the problem so
far. What is the error message? Is 1.jl and 2.jl in the same directory? Do
you manually push the module's directory onto LOAD_PATH, or is it in your
.julia/v0.5 directory?

On my system, running creload("ClobberingReload") from any directory
works, and that's a module that includes two files.


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#3 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ADP2XeBNK6SVR50RBhe9_GLnUqNP4jIGks5qvE-JgaJpZM4KKjWR
.

@cstjean
Copy link
Owner

cstjean commented Sep 30, 2016

Yes I manually add the module path to LOAD_PATH . In .julia/v0.5 I have only code that is not mine .

My personal project is also on LOAD_PATH (not in the "current" directory), contains 4-5 includes, and creload works fine so far. I just tried with another module in another directory, same results. I'm happy to help debug anything, if you have time to write down a more complete example. Is the problem with autoreload?

Anyway .. I was looking through your code and I think that if you include macro exapansion (macroexpand() ). In the code that parses all expressions . It can maybe help overcome some limitations still existent in your clobber method.

If the include statements are part of a macroexpansion then yes, they won't be detected by autoreloading, but creload should work fine.

Macroexpanding everything sounds appealing, but in my experience it's a huge rabbit hole, and I would rather not go there. In particular, if the module is defined and called in the same file, merely calling macroexpand won't work. We would have to process the macro definition first somehow, and that gets ugly fast.

BTW, if you're creloading many modules, it's important to creload them in import order. I'll fix that once I support dependencies for autoreload.

I am working on a project(AR glasses) I have more than one conceptual package - HW rendering tracking etc.

Cool, where do you work?

@TsurHerman
Copy link
Author

It seems that the problem is related to Atom and Juno and not to your code
.. If I run it from the julia REPL it works as expected.
But if I am running it from within Atom (using Juno console) then I am
getting a message:
could not open file C:\Users\tsurh.atom\packages\julia-client\script\B.jl

where B is a file under folder test\ in my project directory ..

But if I am using the julia builtin REPL it works as expected and finds
DirToMyProject\test\B.jl correctly.

Cool, where do you work?

I recently started working with a new startup, in Israel, bringing AR into
the operation room (surgical operation)

On Fri, Sep 30, 2016 at 5:44 PM, Cédric St-Jean notifications@github.com
wrote:

Yes I manually add the module path to LOAD_PATH . In .julia/v0.5 I have
only code that is not mine .

My personal project is also on LOAD_PATH (not in the "current" directory),
contains 4-5 includes, and creload works fine so far. I just tried with
another module in another directory, same results. I'm happy to help debug
anything, if you have time to write down a more complete example. Is the
problem with autoreload?

Anyway .. I was looking through your code and I think that if you include
macro exapansion (macroexpand() ). In the code that parses all expressions
. It can maybe help overcome some limitations still existent in your
clobber method.

If the include statements are part of a macroexpansion then yes, they
won't be detected by autoreloading, but creload should work fine.

Macroexpanding everything sounds appealing, but in my experience it's a
huge rabbit hole, and I would rather not go there. In particular, if the
module is defined and called in the same file, merely calling macroexpand
won't work. We would have to process the macro definition first somehow,
and that gets ugly fast.

BTW, if you're creloading many modules, it's important to creload them in
import order. I'll fix that once I support dependencies for autoreload.

I am working on a project(AR glasses) I have more than one conceptual
package - HW rendering tracking etc.

Cool, where do you work?


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#3 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ADP2XQD7Y-QIAW304qp0YbyrBk7_L3HFks5qvSAygaJpZM4KKjWR
.

@cstjean
Copy link
Owner

cstjean commented Oct 3, 2016

Juno already has some functionality for reloading modules, as you probably know, and it's likely to interfere with ClobberingReload. Could you try running Base.find_in_node_path("YourModuleName", nothing, 1) in atom to see if the directory is correct?

@TsurHerman
Copy link
Author

The directory is correct .. but somehow the whole evaluation is considered
to be evaluated from the Juno server which is in a different path and so it
doesn't follow includes correctly.

For example if I call creload from Juno Repl I get an error
Cannot find "PathToJunoServer/IncludedFileName.jl"

If I call creload by "including" a script to reload all my modules I get an
error
Cannot find "PathToScript/IncludedFileName.jl"

I workaround this by having a macro that prepends the full path to module
includes.

On Monday, 3 October 2016, Cédric St-Jean notifications@github.com wrote:

Juno already has some functionality
https://github.com/JunoLab/atom-julia-client/blob/master/manual/workflow.md
for reloading modules, as you probably know, and it's likely to interfere
with ClobberingReload. Could you try running Base.find_in_node_path("YourModuleName",
nothing, 1) in atom to see if the directory is correct?


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#3 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ADP2Xfdaf0zzm4szYY871sBNXWOJoPngks5qwT7EgaJpZM4KKjWR
.

@cstjean
Copy link
Owner

cstjean commented Oct 4, 2016

@MikeInnes Any idea why this code wouldn't work in Juno-atom? I took the eval trick from your repo... It seems that the eval is not happening in the module's directory (hence the includes are broken), but in Juno's directory.

function creload(mod_name)
    mod_path = Base.find_in_node_path(mod_name, nothing, 1)
    cd(dirname(mod_path)) do
        # this will call readstring(basename(mod_path))
        real_mod_name, code = parse_module_file(basename(mod_path))  
        eval(eval(Main, real_mod_name), :(begin $(code...) end))
    end
end

@MikeInnes
Copy link

MikeInnes commented Oct 4, 2016

include() looks up files relative to Base.source_path(), which represents the current file being loaded (not the pwd, which I suggest not changing at all). I'm not sure why you're not inheriting the source path that Juno sets (which should be the current file or nothing) but either way, when evaluating you should fake the current source file with something like this, e.g.

withpath("/path/to/module.jl") do
  eval(:(include("x.jl"))) # correctly gets "/path/to/x.jl"
  eval(:(@__FILE__)) # also now works correctly, etc.
end

Also, you'll get closer to the original semantics of file loading if you evaluating top-level statements one at a time – that will solve problems with macro definitions and such. I think you can get this as easily as by using Expr(:toplevel, code...) rather than a block.

@cstjean
Copy link
Owner

cstjean commented Oct 4, 2016

Thank you for the detailed answer. @TsurHerman I've implemented Mike's suggestions. If you want to give it a try, Pkg.checkout("ClobberingReload")

@TsurHerman
Copy link
Author

Thanks :-) yes I will give it a try .

@cstjean cstjean changed the title creload does not follow include correctly creload does not follow include correctly in Juno Oct 6, 2016
@cstjean
Copy link
Owner

cstjean commented Nov 6, 2016

@TsurHerman Did it fix the issue?

@TsurHerman
Copy link
Author

Hi! Sorry for the late answer, I think so .. but I am since using auto generated full path , so I can't really tell.

@cstjean
Copy link
Owner

cstjean commented Nov 6, 2016

Ok, I'll close it for now; reopen if it's still an issue.

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

3 participants