spacetelescope / style-guides

An opinionated guide on how we work.
Creative Commons Attribution 4.0 International
55 stars 33 forks source link

Jupyter notebook style guide revision #110

Closed lglattly closed 4 years ago

lglattly commented 4 years ago

This pull request is to revise and update the STScI Jupyter Notebook style guide as part of the Kepler-Focus Lightkurve Tutorial project to create a reconciled tutorial template for the new Lightkurve tutorials as well as for Astropy and STScI tutorials moving forward. This pull request contains copy edits to the text of the Jupyter Notebook style guide as well as revisions and additions to the suggested notebook structure and the example notebook.

Note for @eteq, @kelle, and @barentsen, drawing attention to three questions I had marked by ... in the files:

barentsen commented 4 years ago

@lglattly This looks great to me! :+1:

My personal two cents:

  1. I'd apply the style guide to any tutorial, whether or not it was created using a notebook.
  2. Agreed!
  3. Nothing jumps to mind.
lglattly commented 4 years ago

@eteq, letting you know that @kelle and I discussed my above questions and I resolved them in this last round of commits, but we have some new things to discuss in terms of adjusting the tutorial template structure in the style guide. I've made note of these suggestions in the files using ... and they can be found on these lines in the files changed:

eteq commented 4 years ago

Some additional folks who might want to offer thoughts here if they have bandwidth: @ceb8 @mustaric @orifox @camipacifici

ceb8 commented 4 years ago

I very much agree with moving the imports after the intro. An import list doesn't give the user any info about whether or not the tutorial is what they are looking for, but the intro very much does.

I disagree with removing the additional resources section. I agree that they should be linked within the main body as they come up, however additionally having a list at the end makes the links easy to find without having to read through the entire body of the notebook. I think it is likely that these notebooks are used not only as tutorials to be worked through sequentially, but also as reference documents to return to when associated concepts and techniques arise, and in that second context being able to go directly to the list of related additional resources can very helpful.

lglattly commented 4 years ago

Thank you for the feedback @eteq and @ceb8! There seems to be consensus that moving Imports to after Introduction is a good change, and that moving Loading Data and File Information into the Main Content works, but that the Additional Resources section should not be eliminated.

So in this last round of commits, I revised the first paragraph as per the suggestion from @eteq and I moved the Imports section to after the Introduction. I also moved the Loading Data and File Information sections into the Main Content and added an additional explanation about using descriptive headings for these sections and using them at the same time data will be used if possible. I left the Additional Resources section but noted it is optional, and added some language encouraging authors to weave their resource links into their narrative, but with the option of having an Additional Resources section for resources that may not fit cleanly into the narrative.

I made these changes in both the style guide file and the example notebook template. Unless there is any other feedback, I think this PR is finalized and ready to be merged!

sean-lockwood commented 4 years ago

Did you intend the citations section to render the markdown?

image

If so, why the triple-quotes?

image
lglattly commented 4 years ago

@sean-lockwood I was simply mimicking the formatting style that already existed in the file, such as in the "Sections (xN)" section and the "File information" section of the original file. So yes, I intended for the rendered markdown part to look like an example, as in the other sections. But the Citations section can be formatted however makes the most sense. Let me know if I should make any changes!

eteq commented 4 years ago

I'm interpreting @sean-lockwood's :+1: as being fine with as it stands, so I think this is now set to go. Merging, although of course follow-on PRs are always welcome!

lglattly commented 4 years ago

Thanks everyone!