Skip to content

Add displaying and loading of recipe output to the notebook API#957

Merged
nielsdrost merged 24 commits intomasterfrom
api_run_output2
Jan 26, 2021
Merged

Add displaying and loading of recipe output to the notebook API#957
nielsdrost merged 24 commits intomasterfrom
api_run_output2

Conversation

@stefsmeets
Copy link
Contributor

@stefsmeets stefsmeets commented Jan 21, 2021

Description

Hi everyone, this PR extends the API to run recipes in an interactive notebook.

It adds functionality to obtain the output data (data or plots), and display or load them directly in the notebook.

Todo

  • Wrap recipe_output dict and add convenience methods
  • Add shorthand to get items of a type, i.e. recipe_output.get_output_of_type('data')
  • Elaborate/update on documentation
  • Fine-tune TaskOutput usage (__repr__ / rename .filename attribute)

Before you get started

Checklist


To help with the number pull requests:

@stefsmeets stefsmeets requested a review from Peter9192 January 21, 2021 12:17
@stefsmeets
Copy link
Contributor Author

I just realized we do not depend on xarray in ESMValCore. Just wondering if having a .load_xarray method warrants adding it to the list of dependencies. Afaik ESMValTool has it in its dependencies after all.

Copy link
Contributor

@Peter9192 Peter9192 left a comment

Choose a reason for hiding this comment

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

Very cool stuff @stefsmeets, this is really impressive already! Here's a few general comments:

  • The documentation (before the API reference) is a bit limited, and I'm not sure if I could readily use it. The notebook helped me to understand how to the items from the output. A slightly more elaborate example would be nice.
  • Would it be simple enough to make shorthand methods for these intimidating lines in the example notebook like data_items = [item for tasks in recipe_output.values() for item in tasks if item.kind == 'data']? Then I think that's worth adding.
  • How much effort would it be to have a nice markdown rendering of the output, like you did for the Recipe Info? I think that could potentially be awesome as well.

Co-authored-by: Peter Kalverla <peter.kalverla@gmx.com>
@stefsmeets
Copy link
Contributor Author

stefsmeets commented Jan 21, 2021

Thanks @Peter9192

* The documentation (before the API reference) is a bit limited, and I'm not sure if I could readily use it. The notebook helped me to understand how to the items from the output. A slightly more elaborate example would be nice.

Yeah, I agree. I am collecting the notebooks I'm developing so that they can be used to support the documentation. I will try to see what I can do to add some more information.

* Would it be simple enough to make shorthand methods for these intimidating lines in the example notebook like `data_items = [item for tasks in recipe_output.values() for item in tasks if item.kind == 'data']`? Then I think that's worth adding.

This would mean turning recipe output into a class, and then add a method like .get_output_of_type('data'). I can do that 👍

* How much effort would it be to have a nice markdown rendering of the output, like you did for the Recipe Info? I think that could potentially be awesome as well.

Technically it is not too difficult. I'm already using HTML so that I can render the caption as well as the image. This can be extended with other attributes. I think you can do inline html in markdown, which may be a another way to do it.

I actually started adding the authors, but I stopped because I was worried of losing scope for this PR. It would require going through the attributes and presenting them in a nice way, including stuff like authors and references (which we already have), but also realms and domains. I think this PR puts a good baseline to continue from.

This enables nicer output in the notebook, and convenience functions.
Also lays the foundation for other functionality, like a `.report`
method that can write an html document etc.
@stefsmeets
Copy link
Contributor Author

It was a bit of work, but I added wrappers for the task and recipe output to make it a bit easier to work with these objects. This also lays the foundation to do stuff like recipe_output.report() and get a nice html representation. I think this is something we may want in the future (but not in this PR!).

@stefsmeets
Copy link
Contributor Author

Hi @Peter9192 , I implemented all your feedback. If you approve, I think this PR is ready to be merged!

@stefsmeets stefsmeets requested a review from Peter9192 January 22, 2021 14:51
@stefsmeets stefsmeets marked this pull request as ready for review January 22, 2021 14:55
Copy link
Contributor

@Peter9192 Peter9192 left a comment

Choose a reason for hiding this comment

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

Hey Stef! This is starting to be very very cool! I have a couple of follow-up comments/questions, but all-in-all I'm very happy with this PR.

On codacy: shall we just move the imports to the top?

Co-authored-by: Peter Kalverla <peter.kalverla@gmx.com>
@stefsmeets
Copy link
Contributor Author

stefsmeets commented Jan 25, 2021

On codacy: shall we just move the imports to the top?

I moved iris, but I can't do that for xarray, because it is imported as a convenience function, but we don't have it in our dependencies (see #957 (comment)).

@stefsmeets
Copy link
Contributor Author

This PR is ready to be merged @nielsdrost !

@nielsdrost nielsdrost self-requested a review January 25, 2021 13:38
@stefsmeets stefsmeets requested a review from nielsdrost January 25, 2021 14:59
@nielsdrost nielsdrost merged commit 702991f into master Jan 26, 2021
@nielsdrost nielsdrost deleted the api_run_output2 branch January 26, 2021 09:39
@jvegreg jvegreg added the api Notebook API label Feb 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api Notebook API

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants