Lazy loading for Gtk #236
Lazy loading for Gtk #236
Conversation
Update Fork
Minor fix
|
Hey @Sov-trotter - thanks for continuing to look at this. I am still getting the same error that I got last time being this along with some warnings about overwriting: (@v1.5) pkg> activate .
Activating environment at `/home/src/Projects/javis/Project.toml`
julia> include("view.jl")
[ Info: Precompiling Javis [78b212ba-a7f9-42d4-b726-60726080707e]
WARNING: Method definition __init__() in module Javis at /home/src/Projects/javis/src/Javis.jl:101 overwritten at /home/src/Projects/javis/src/Javis.jl:441.
** incremental compilation may be fatally broken for this module **
ERROR: LoadError: UndefVarError: _javis_viewer not defined
Stacktrace:
[1] javis(::Video, ::Array{Action,1}; framerate::Int64, pathname::String, liveview::Bool, tempdirectory::String) at /home/src/Projects/javis/src/Javis.jl:227
[2] top-level scope at /home/src/Projects/javis/view.jl:94
[3] include(::String) at ./client.jl:457
[4] top-level scope at REPL[2]:1
in expression starting at /home/src/Projects/javis/view.jl:94Here is the code I was using: using Javis
using Random
function ground(args...)
background("white")
sethue("black")
end
function draw_line(p1 = O, p2 = O, color = "black", action = :stroke, edge = "solid")
sethue(color)
setdash(edge)
line(p1, p2, action)
end
function circ(p = O, color = "black", action = :fill, radius = 25, edge = "solid")
sethue(color)
setdash(edge)
circle(p, radius, action)
end
function info_box(video, action, frame)
fontsize(12)
box(140, -210, 170, 40, :stroke)
text("10-20 EEG Array Readings", 140, -220, valign = :middle, halign = :center)
text("t = $(frame)s", 140, -200, valign = :middle, halign = :center)
end
function electrode(
p = O,
fill_color = "white",
outline_color = "black",
action = :fill,
radius = 25,
circ_text = "",
)
sethue(fill_color)
circle(p, radius, :fill)
sethue(outline_color)
circle(p, radius, :stroke)
text(circ_text, p, valign = :middle, halign = :center)
end
electrode_locations = [
O,
Point(-70, 0),
Point(70, 0),
Point(-140, 0),
Point(140, 0),
Point(0, 70),
Point(-50, 70),
Point(50, 70),
Point(0, -70),
Point(-50, -70),
Point(50, -70),
Point(115, -80),
Point(-115, -80),
Point(115, 80),
Point(-115, 80),
Point(40, -135),
Point(-40, -135),
Point(-190, -10),
Point(190, -10),
Point(-40, 135),
Point(40, 135),
]
electrode_names = [
"Cz",
"C3",
"C4",
"T3",
"T4",
"Pz",
"P3",
"P4",
"Fz",
"F3",
"F4",
"F8",
"F7",
"T6",
"T5",
"Fp2",
"Fp1",
"A1",
"A2",
"O1",
"O2",
]
radius = 15
indicators = ["tomato", "darkolivegreen1", "gold1", "white"]
demo = Video(500, 500)
javis(
demo,
[
BackgroundAction(1:10, ground),
Action(
:inside_circle,
(args...) -> circ(O, "black", :stroke, 140, "longdashed"),
),
Action(:head, (args...) -> circ(O, "black", :stroke, 170)),
Action(
:vert_line,
(args...) ->
draw_line(Point(0, -170), Point(0, 170), "black", :stroke, "longdashed"),
),
Action(
:horiz_line,
(args...) ->
draw_line(Point(-170, 0), Point(170, 0), "black", :stroke, "longdashed"),
),
Action(
:electrodes,
(args...) ->
electrode.(
electrode_locations,
rand(indicators, length(electrode_locations)),
"black",
:fill,
radius,
electrode_names,
),
),
Action(:info, info_box),
],
pathname = "eeg.gif",
framerate = 1,
liveview = true
)It seems like you are getting closer based on the grid worlds issue you found here: JuliaReinforcementLearning/GridWorlds.jl@f46e78c Maybe ask @findmyway for some help? He used Javis to make the logo for JuliaReinforcementLearning. 😄 |
|
Yeah. I am digging in a bit more since we are now clear about what we want and also that we have a use case. I'll let you know once I make sure that things work well on my end. :))) |
|
Thanks for @TheCedarPrince 's invitation. Hi @Sov-trotter , I notice that you've commented out some tests. I guess some important parts are missed. To keep tests working as usual, you need to add the Another way is to create a new environment inside of tests folder. (By creating a Project.toml file in the tests) . But I forget when is this feature enabled. (I guess since Julia@v1.3) Another tip. Given that GtkReactive relies on GTK, you can simply require using .GtkReactive
using .GtkReactive.GTKSo that one can simply enable those extra features by |
Refactor stuff/work out test deps
Cleanup refs
|
Thank you so much @findmyway for the help. I was able to get things working with the examples @TheCedarPrince quoted but the tests were failing for some reason. For now I fixed it by using Also there are two warnings viz. I am not really sure what can be done on this side but I'd love to work on any suggestions. :) |
Codecov Report
@@ Coverage Diff @@
## master #236 +/- ##
==========================================
- Coverage 96.89% 96.24% -0.65%
==========================================
Files 19 19
Lines 676 719 +43
==========================================
+ Hits 655 692 +37
- Misses 21 27 +6
Continue to review full report at Codecov.
|
|
Thanks @Sov-trotter and @findmyway The warnings appear because both GtkReactive and Javis export I think you can remove the test extras from the Project.toml as you have them in Additionally I am wondering whether you deleted the reference images. Maybe you can specify why the test failed. Is it because of some reference images? (This shouldn't happen 😟 ) |
|
Oh. Sorry for that, since the tests were successful for the first time on my machine, I though those were supposed to be ignored. |
|
You probably want to use something like: in this case Refs are the reference images which stay constant such that the newly computed images are checked against them. |
|
Great work. Now is it somehow possible that the user doesn't have to write |
|
Yeah was thinking of the same. That side would be kinda different from the requires route I guess. One way I am thinking of is where the true flag is checked for? Not sure though |
|
Maybe you can double check that this works: if liveview == true
@eval import .GtkReactive
@eval import .GtkReactive: slider, signal
@eval using .GtkReactive.Gtk
_javis_viewer(video, length(frames), actions)
return "Live preview started."
end(inside and then remove the Some extras: I'm not sure about |
Wikunia
added
the
hacktoberfest-accepted
|
Oh cool. What I wanted to try was while the file would remain the same, when I call the function with |
Update changelog
|
Yours might fail because it tries to load it in the current working directory instead of the javis src directory? Unfortunately mine doesn't work. Feels a bit weird to write that as the user 😕 |
Sorry you mean with Requires or the if liveview=true condition method? |
|
Sorry I mean if the user has to write: like it is currently the case with this PR. |
|
Ah yeah. But I don't think that will bring up the warnings since we have it fixed inhere https://github.com/Wikunia/Javis.jl/blob/f5c299aabf8ab6dcf23b0698c9199344ef35bf12/src/javis_viewer.jl#L1-L3. The main role of |
|
I think there is a problem if the user writes |
|
Sorry didn't get you? |
|
Sorry my explanation is probably kinda lacking 😄 using Javis
using GtkReactive
function drawing(args...)
label() # should be Javis but is ambiguous as GtkReactive also exports it, right?
endI hope this makes clear what I'm worried about. Anyway best would be if the user doesn't need to write |
|
Oh. I think the warning can be avoided by using a qualification ? But yeah this seems to take us down to more such warnings in the future. :/ |
|
It can be avoided by using |
|
Hey @Sov-trotter - I posted about this on the Julia Discourse and it seems like we may have a few potential answers (see here: https://discourse.julialang.org/t/dynamic-code-loading-to-use-packages-on-demand/48655). Hopefully, one of these can address this problem. |
|
Hey @Sov-trotter do you still want to work on this or should I take it over? Thanks! |
|
Yeah. I'd love to. Sorry I am a bit slow due to some other commitments. I'll get it done soon. :) |
|
No hurry @Sov-trotter We probably need at least 10 more days to bring v0.3 and I think this issue can't be added before that anyway as it needs a new dependency. |
Still have the world age error
|
So following up on the discussion over discourse, I am still getting the world age error. Not sure what's incorrect though. :/ |
|
Hey @Wikunia and @Sov-trotter - I checked this out again and I am really stuck on what to do with this issue. I am debating about closing it altogether sadly. 😞 |
|
I feel like it's not strictly necessary but appreciate the effort @Sov-trotter . |
|
Okay. Sorry though I am not able to commit time rn(caught up in academic stuff currently), I think till mid-december. I think you can close it for now. We can however revisit this later as the package grows. |
4 participants
PR Checklist
If you are contributing to
Javis.jl, please make sure you are able to check off each item on this list:CHANGELOG.mdwith whatever changes/features I added with this PR?Project.toml+ set an upper bound of the dependency (if applicable)?testdirectory (if applicable)?Link to relevant issue(s)
Closes #193
How did you address these issues with this PR? What methods did you use?
Continuation of #225