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

%import magic breaks global variables #5961

Closed
altavir opened this issue Sep 11, 2017 · 5 comments
Closed

%import magic breaks global variables #5961

altavir opened this issue Sep 11, 2017 · 5 comments
Assignees
Milestone

Comments

@altavir
Copy link

altavir commented Sep 11, 2017

Like that:

a = 2

2

%import static java.lang.Math.PI
a*PI

groovy.lang.MissingPropertyException: No such property: a for class: script15051208463782932642
at script15051208463782932642.run(script15051208463782932642.groovy:1)
at com.twosigma.beakerx.groovy.evaluator.GroovyCodeRunner.run(GroovyCodeRunner.java:71)

I do not know how imports are implemented, but probably you create new shell instance with new compilation customization. In that case, you have to copy bindings content into new shell.

@scottdraves
Copy link
Contributor

confirmed. thx for the report, i think your diagnosis is correct.
in the meantime, the workaround would be to run all %imports first.

@scottdraves scottdraves added this to the 0.4 milestone Sep 11, 2017
@altavir
Copy link
Author

altavir commented Sep 11, 2017

Done it already. Copping the context should pretty simple. You do not even need to clone it since it is stored in the separate Binding variable which could be passes to shell constructor.

@scottdraves
Copy link
Contributor

Do you mean you have code that fixes it? Would be awesome if you could make a PR...

@altavir
Copy link
Author

altavir commented Sep 11, 2017

Just looked through the code. It is implemented in a slightly different way, You do not use the shell object explicitly, instead you create a separate script for each cell (script parser creates the shell under the hood). In my experience this way you loose performance, but it is probably the best solution for notebook since you do not know what cell to expect next.

The problem is probably in this line. It flushes the whole binding on classloader reset. One can try to just comment it, but I do not know why it was done in the first place, so I do not know if it will break anything.

@lmitusinski
Copy link
Contributor

This one seems to be fixed with:
#5965

@LeeTZ LeeTZ closed this as completed Jul 17, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants