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

Issue 1185 jupyter notebook #59

Open
wants to merge 22 commits into
base: master
Choose a base branch
from

Conversation

RichieBrady
Copy link

@PekkaSavolainen This branch is up to date with the latest from today. The dependency for papermill is still in spine items, I am starting to move this to the engine. Note that this requires an update to the toolbox. I will make a pull request for that now.

@ptsavol
Copy link
Member

ptsavol commented Apr 12, 2021

Ok thanks, got it working and ran my first .ipynb file. I'll test some more and post comments tomorrow.

@ptsavol
Copy link
Member

ptsavol commented Apr 13, 2021

About the icon. How about this:

https://fontawesome.com/icons/book?style=solid

@ptsavol
Copy link
Member

ptsavol commented Apr 13, 2021

There's been some changes to the Tools spec editor window. Do you mind updating the Notebook spec editor window so they are consistent.

EDIT
Nevermind, this new Notebook spec editor has Jupyter Notebook input variables and Jupyter Notebook output variables lists, which are not in Tool spec editor of course. There's room for improvement but there's going to be changes to the Tool spec editor in the near future, so I think it's ok to make the notebook spec editor window more consistent after the merge.

@ptsavol
Copy link
Member

ptsavol commented Apr 13, 2021

@ererkka, @soininen, @manuelma

Anybody interested in testing the functionality/usability of this new item? I can check the code but I'm not a Jupyter Notebook user so I need input about the user experience.

@ptsavol
Copy link
Member

ptsavol commented Apr 13, 2021

The logger in papermill.execute_notebook() is the regular python logging module, so the [INFO] and [DEBUG] messages go to the console (i.e. where print() messages go). I was wondering what is your plan for these?

  1. Can we use our own LoggerInterface to catch the messages coming from papermill.execute_notebook() so that we could print them to our Execution Log QTextBrowser?

  2. The messages seem to be in a format that our kernel manager could interpret. Can we execute a Python notebook using papermill in our own Python Console?

@manuelma
Copy link
Collaborator

I definitely want to give this one a try but I'm a bit busy at the moment. I wouldn't mind if you merge it before I get to try it out, too.

@soininen
Copy link
Contributor

papermill package seems to be missing from Toolbox's setup.py causing the unit tests to fail.

@ptsavol
Copy link
Member

ptsavol commented Apr 13, 2021

papermill package seems to be missing from Toolbox's setup.py causing the unit tests to fail.

Shouldn't it be in spine_engine setup.py instead?

@soininen
Copy link
Contributor

Shouldn't it be in spine_engine setup.py instead?

I missed the fact that the package is imported only in the executable item so I guess spine_engine it is, then.

@ptsavol
Copy link
Member

ptsavol commented Apr 13, 2021

@RichieBrady FYI to make testing different spine_items branches easier, I moved category.py from toolbox to spine_items. Please update the spine_items/category.py for the notebook branch. In addition, please add this "item_type": ItemInfo.item_type() to notebook_specifications.py to_dict() function. After these updates, there's no need to change anything in ui_main.py in Spine Toolbox and the PR spine-tools/Spine-Toolbox#1365 can be about the new tests and the 'preferred' spine_items version bump. (if I am correct here, of course).

EDIT

And of course I did not test it enough. You need to add an additional new_spec_dict["item_type"] = ItemInfo.item_type()
to notebook_specification_editor_window.py _save() method. Sorry about the hassle.

@ptsavol
Copy link
Member

ptsavol commented Apr 14, 2021

The code needs a bit of house cleaning. There are some leftover methods from Tool that are surely not used by Notebook (for example, upgrade_v1_to_v2() in notebook.py). Also, there are a lot of docstrings that refer to Tool when they should say Notebook.

@soininen
Copy link
Contributor

I wouldn't mind if my name was dropped from the :author: fields in the headers of new files. I don't really feel I authored anything here. (I'd rather drop the :author: and :date: fields in every single file since we live in a version controlled world but that is another discussion for another time.)

spec_model_index = self._toolbox.specification_model.specification_index(self.specification().name)
spec_menu = NotebookSpecificationMenu(self._toolbox, spec_model_index)
actions = {a.text(): a for a in spec_menu.actions()}
self._actions = [actions["Edit specification"], actions["Open jupyter notebook"]]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Context-menu don't work because of a typo here. Should be Jupyter (capital J)

@ptsavol
Copy link
Member

ptsavol commented Apr 14, 2021

Execution in Source directory fails with this traceback

2021-04-14 10:47:18 INFO: Input Notebook:  shopping_list.ipynb
2021-04-14 10:47:18 INFO: Output Notebook: C:/Users/ttepsa/OneDrive - Teknologian Tutkimuskeskus VTT/Documents/SpineToolboxProjects/Notebook project/.spinetoolbox/items/notebook_2\shopping_list_out.ipynb
Exception in thread Thread-8:
Traceback (most recent call last):
  File "C:\Users\ttepsa\.julia\conda\3\envs\notebook\lib\site-packages\papermill\iorw.py", line 197, in read
    json.loads(path)
  File "C:\Users\ttepsa\.julia\conda\3\envs\notebook\lib\json\__init__.py", line 348, in loads
    return _default_decoder.decode(s)
  File "C:\Users\ttepsa\.julia\conda\3\envs\notebook\lib\json\decoder.py", line 337, in decode
    obj, end = self.raw_decode(s, idx=_w(s, 0).end())
  File "C:\Users\ttepsa\.julia\conda\3\envs\notebook\lib\json\decoder.py", line 355, in raw_decode
    raise JSONDecodeError("Expecting value", s, err.value) from None
json.decoder.JSONDecodeError: Expecting value: line 1 column 1 (char 0)

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "C:\Users\ttepsa\.julia\conda\3\envs\notebook\lib\threading.py", line 926, in _bootstrap_inner
    self.run()
  File "C:\Users\ttepsa\.julia\conda\3\envs\notebook\lib\threading.py", line 870, in run
    self._target(*self._args, **self._kwargs)
  File "C:\Users\ttepsa\.julia\conda\3\envs\notebook\lib\site-packages\spine_engine\spine_engine.py", line 400, in _execute_item_filtered
    item_success = item.execute(filtered_forward_resources, filtered_backward_resources)
  File "C:\Users\ttepsa\.julia\conda\3\envs\notebook\lib\site-packages\spine_items\notebook\executable_item.py", line 231, in execute
    return_code = self._notebook_instance.execute()
  File "C:\Users\ttepsa\.julia\conda\3\envs\notebook\lib\site-packages\spine_items\notebook\notebook_instance.py", line 94, in execute
    return self._console_execute()
  File "C:\Users\ttepsa\.julia\conda\3\envs\notebook\lib\site-packages\spine_items\notebook\notebook_instance.py", line 105, in _console_execute
    parameters=self._nb_parameters
  File "C:\Users\ttepsa\.julia\conda\3\envs\notebook\lib\site-packages\papermill\execute.py", line 90, in execute_notebook
    nb = load_notebook_node(input_path)
  File "C:\Users\ttepsa\.julia\conda\3\envs\notebook\lib\site-packages\papermill\iorw.py", line 410, in load_notebook_node
    nb = nbformat.reads(papermill_io.read(notebook_path), as_version=4)
  File "C:\Users\ttepsa\.julia\conda\3\envs\notebook\lib\site-packages\papermill\iorw.py", line 101, in read
    notebook_metadata = self.get_handler(path).read(path)
  File "C:\Users\ttepsa\.julia\conda\3\envs\notebook\lib\site-packages\papermill\iorw.py", line 201, in read
    raise e
  File "C:\Users\ttepsa\.julia\conda\3\envs\notebook\lib\site-packages\papermill\iorw.py", line 191, in read
    with io.open(path, 'r', encoding="utf-8") as f:
FileNotFoundError: [Errno 2] No such file or directory: 'shopping_list.ipynb'

@ptsavol
Copy link
Member

ptsavol commented Apr 14, 2021

A nice to have improvement. Double-clicking a Notebook item on Design View should open the notebook spec editor window (Like a Tool does).

@codecov-commenter
Copy link

codecov-commenter commented Apr 20, 2021

Codecov Report

Merging #59 (16d0b13) into master (e66afe5) will decrease coverage by 0.38%.
The diff coverage is 56.40%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #59      +/-   ##
==========================================
- Coverage   60.57%   60.19%   -0.39%     
==========================================
  Files         241      265      +24     
  Lines       18244    19992    +1748     
==========================================
+ Hits        11052    12034     +982     
- Misses       7192     7958     +766     
Impacted Files Coverage Δ
spine_items/notebook/executable_item.py 16.90% <16.90%> (ø)
...ok/widgets/notebook_specification_editor_window.py 18.28% <18.28%> (ø)
spine_items/notebook/notebook_instance.py 19.35% <19.35%> (ø)
spine_items/notebook/output_resources.py 23.52% <23.52%> (ø)
spine_items/notebook/widgets/custom_menus.py 36.11% <36.11%> (ø)
spine_items/notebook/utils.py 37.50% <37.50%> (ø)
spine_items/notebook/commands.py 50.00% <50.00%> (ø)
spine_items/notebook/notebook_icon.py 60.78% <60.78%> (ø)
spine_items/notebook/notebook.py 60.93% <60.93%> (ø)
...pine_items/notebook/widgets/add_notebook_widget.py 63.63% <63.63%> (ø)
... and 39 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e66afe5...16d0b13. Read the comment docs.

@manuelma
Copy link
Collaborator

It looks like @RichieBrady is unable to continue this? How do we proceed then? @PekkaSavolainen @soininen @ererkka @jkiviluo ?

@ptsavol
Copy link
Member

ptsavol commented Sep 14, 2021

To be continued in v0.8. See spine-tools/Spine-Toolbox#1185.

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

Successfully merging this pull request may close these issues.

5 participants