gnu-octave/pkg-jupyter-notebook

clear all is not allowed

asereq opened this issue · 4 comments

asereq commented

Hi!
Great work, thanks for the package!

It seems like if the notebook has a "clear all" line,
the "clear" clean variables of the class itself:

  error: 'obj' undefined near line 545, column 545
  error: called from
      evalCode at line 545 column 7
      run at line 389 column 20

My workaround is to delete the line before the execution, but I think it could be a better solution.
Thanks again!

siko1056 commented

Thanks for reporting the issue. @Abdallah-Elshamy do you have an idea how to deal with it?

Abdallah-Elshamy commented

Thanks for the report!

@siko1056 , Since clear all removes all the stored variables, it also removes the jupyter_notebook object itself. When the evalCode function tries to save the context after that, the error happens.

A rough solution for that would be detecting the clear all or clear and add the -x option to it to prevent the object from being deleted. However, this will still affect the variables defined outside of the notebook.

To provide better isolation for the notebook variables, it is better to parse the clear commands and remove the deleted variable from the context that we are storing in the jupyter_notebook object. However, this parsing will slow down performance. There may be also other options that I am not considering. What do you think?

siko1056 commented

Thanks for the brainstorming @Abdallah-Elshamy . I also thought about parsing for clear, but I think there are too many ugly cases of calling clear to be considered, this might not be a stable solution.

I had a very elegant solution, see #7. However, there is https://savannah.gnu.org/bugs/?62077 😓 Declaring everything as public as last resort violates OOP a lot 😅

Abdallah-Elshamy commented

Thanks for the brainstorming @Abdallah-Elshamy . I also thought about parsing for clear, but I think there are too many ugly cases of calling clear to be considered, this might not be a stable solution.

I agree. Too many edge cases to handle and even if we got all of them right, the performance would be worse.

I had a very elegant solution, see #7.

Neat!

However, there is https://savannah.gnu.org/bugs/?62077 😓 Declaring everything as public as last resort violates OOP a lot 😅

I don't think that will be a good idea too. I believe that maintaining OOP principles is more important than handling this edge case. I guess this will have to wait until this bug is solved 😓.

  • ❤️1