desihub / desitest

Testing coordination for DESI code
BSD 3-Clause "New" or "Revised" License
0 stars 1 forks source link

Include notebook outputs? #16

Closed weaverba137 closed 6 years ago

weaverba137 commented 6 years ago

Before I submit a PR for the 18.6 minitest notebook, I had a question. When submitting a PR involving a notebook, it might be easier to clear all of the outputs first, making it easier to see changes to the code in the cells, rather than have the diff full of changes to the output. Is this something worth doing?

sbailey commented 6 years ago

Good question.

In this case, let's keep the outputs in the saved notebook to make it easier to use it as a read-only reference of what was done, without having to re-run the entire notebook. I agree that this makes it difficult to evaluate diffs, but the usefulness of the fully populated reference notebook outweighs that in my opinion. i.e. if you want to compare two notebooks without their outputs, it is easier to remove the unwanted outputs than it is to put them back (3 hours and 1500 CPU hours later...)

Other options to consider in the future:

Related: I would also keep the outputs in the tutorial notebooks for the same reason, though stripping them from the inspector notebook makes sense because the dynamic bokeh/javascript viewer doesn't render in a read-only view on github anyway.

weaverba137 commented 6 years ago

OK, sounds good.