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

Add cell_execute_result to TestbookNotebookClient #69

Closed
fcollonval opened this issue Nov 6, 2020 · 2 comments · Fixed by #71
Closed

Add cell_execute_result to TestbookNotebookClient #69

fcollonval opened this issue Nov 6, 2020 · 2 comments · Fixed by #71

Comments

@fcollonval
Copy link
Member

Similarly to cell_output_text,

testbook/testbook/client.py

Lines 137 to 146 in 957cc4d

def cell_output_text(self, cell) -> str:
"""
Return cell text output
"""
cell_index = cell
if isinstance(cell, str):
cell_index = self._cell_index(cell)
return self._output_text(self.nb['cells'][cell_index])

it would be nice to have a method cell_execute_result.

What do you think?

@rohitsanj
Copy link
Member

rohitsanj commented Nov 7, 2020

Yes, for sure. However, there is an internal static method - TestbookNotebookClient._execute_result which does the job.

Perhaps we could implement a new function cell_execute_result which would internally call _execute_result. Let me know if you would like to take this up @fcollonval 👍

@fcollonval
Copy link
Member Author

Let me know if you would like to take this up @fcollonval 👍

Sure I'll push a PR next week.

fcollonval pushed a commit to fcollonval/testbook that referenced this issue Nov 9, 2020
rohitsanj pushed a commit that referenced this issue Nov 10, 2020
* Add `cell_execute_result` to `TestbookNotebookClient`
Fixes #69

* Fix linter

* More linter fix

* Found the trailing space

* Take into account review comments

* Forget renaming in test...

Co-authored-by: Frédéric Collonval <frederic.collonval@ariadnext.com>
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 a pull request may close this issue.

2 participants