Skip to content

Commit

Permalink
Update template to latest workflows and structure
Browse files Browse the repository at this point in the history
  • Loading branch information
Michael Gough committed Mar 20, 2024
1 parent 8a5d9e7 commit f19ee4f
Show file tree
Hide file tree
Showing 35 changed files with 785 additions and 298 deletions.
29 changes: 29 additions & 0 deletions .github/ISSUE_TEMPLATE/blank_issue_template.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
name: "Open a new issue"
description: "Have a question or an issue? Let us know here."
title: "Enter your issue title here"
labels: []
body:
- type: markdown
attributes:
value: "## Welcome!"
- type: markdown
attributes:
value: >-
We value your input and are here to help. Please use this space to describe your question,
issue, or suggestion with as much detail as possible. Good descriptions include
specific steps to reproduce the issue, documentation of the observed versus expected behavior,
and any relevant screenshots or error messages. Your detailed account helps us to address
your concerns promptly and improve the `jdat-notebooks` for everyone.
- type: input
id: notebook-name
attributes:
label: "Notebook Name:"
placeholder: "Enter the name of the notebook here..."
validations:
required: true
- type: textarea
id: issue-description
attributes:
label: Description
validations:
required: true
4 changes: 4 additions & 0 deletions .github/ISSUE_TEMPLATE/config.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
contact_links:
- name: Question/Help/Support
url: https://jwsthelp.stsci.edu
about: "JWST Help Desk"
54 changes: 54 additions & 0 deletions .github/PULL_REQUEST_TEMPLATE.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
This notebook checklist has been made available to us by the [the Notebooks For All team](https://github.com/Iota-School/notebooks-for-all/blob/main/resources/event-hackathon/notebook-authoring-checklist.md).
Its purpose is to serve as a guide for both the notebook author and the technical reviewer highlighting critical aspects to consider when striving to develop an accessible and effective notebook.

### The First Cell

- [ ] The title of the notebook in a first-level heading (eg. `<h1>` or `# in markdown`).
- [ ] A brief description of the notebook.
- [ ] A table of contents in an [ordered list](https://www.markdownguide.org/basic-syntax/#ordered-lists) (`1., 2.,` etc. in Markdown).
- [ ] The author(s) and affiliation(s) (if relevant).
- [ ] The date first published.
- [ ] The date last edited (if relevant).
- [ ] A link to the notebook's source(s) (if relevant).

### The Rest of the Cells

- [ ] There is only one H1 (`#` in Markdown) used in the notebook.
- [ ] The notebook uses other [heading tags](https://www.markdownguide.org/basic-syntax/#headings) in order (meaning it does not skip numbers).

## Text

- [ ] All link text is descriptive. It tells users where they will be taken if they open the link.
- [ ] All acronyms are defined at least the first time they are used.
- [ ] Field-specific/specialized terms are used when needed, but not excessively.

## Code

- [ ] Code sections are introduced and explained before they appear in the notebook. This can be fulfilled with a heading in a prior Markdown cell, a sentence preceding it, or a code comment in the code section.
- [ ] Code has explanatory comments (if relevant). This is most important for long sections of code.
- [ ] If the author has control over the syntax highlighting theme in the notebook, that theme has enough color contrast to be legible.
- [ ] Code and code explanations focus on one task at a time. Unless comparison is the point of the notebook, only one method for completing the task is described at a time.

### Images

- [ ] All images (jpg, png, svgs) have an [image description](https://www.w3.org/WAI/tutorials/images/decision-tree/). This could be
- [ ] [Alt text](https://developer.mozilla.org/en-US/docs/Web/API/HTMLImageElement/alt) (an `alt` property)
- [ ] [Empty alt text](https://developer.mozilla.org/en-US/docs/Web/API/HTMLImageElement/alt#decorative_images) for decorative images/images meant to be skipped (an `alt` attribute with no value)
- [ ] Captions
- [ ] If no other options will work, the image is decribed in surrounding paragraphs.

- [ ] Any [text present in images](https://www.w3.org/WAI/WCAG21/Understanding/images-of-text.html) exists in a text form outside of the image (this can be alt text, captions, or surrounding text.)

### Visualizations

- [ ] All visualizations have an image description. Review the previous section, Images, for more information on how to add it.
- [ ] [Visualization descriptions](http://diagramcenter.org/specific-guidelines-e.html) include
- [ ] The type of visualization (like bar chart, scatter plot, etc.)
- [ ] Title
- [ ] Axis labels and range
- [ ] Key or legend
- [ ] An explanation of the visualization's significance to the notebook (like the trend, an outlier in the data, what the author learned from it, etc.)

- [ ] All visualizations and their parts have [enough color contrast](https://www.w3.org/WAI/WCAG21/Understanding/contrast-minimum.html) ([color contrast checker](https://webaim.org/resources/contrastchecker/)) to be legible. Remember that transparent colors have lower contrast than their opaque versions.
- [ ] All visualizations [convey information with more visual cues than color coding](https://www.w3.org/WAI/WCAG21/Understanding/use-of-color.html). Use text labels, patterns, or icons alongside color to achieve this.
- [ ] All visualizations have an additional way for notebook readers to access the information. Linking to the original data, including a table of the data in the same notebook, or sonifying the plot are all options.
43 changes: 43 additions & 0 deletions .github/helpers/nb_flake8_magic.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
{
"cells": [
{
"cell_type": "markdown",
"metadata": {},
"source": [
"<p style=\"font-size:200%; color:#e56020; background-color:#1d1160;\"><b><i>Reviewer note:</i> Begin PEP8 check cells (delete below when finished)</b></p>"
]
},
{
"cell_type": "code",
"execution_count": null,
"metadata": {},
"outputs": [],
"source": [
"# disable all imported packages' loggers\n",
"import logging\n",
"logging.root.manager.loggerDict = {}"
]
},
{
"cell_type": "code",
"execution_count": null,
"metadata": {},
"outputs": [],
"source": [
"# enable PEP8 checker for this notebook\n",
"%load_ext pycodestyle_magic\n",
"%flake8_on --ignore E261,E501,W291,W293\n",
"\n",
"# only allow the checker to throw warnings when there's a violation\n",
"logging.getLogger('flake8').setLevel('ERROR')\n",
"logging.getLogger('stpipe').setLevel('ERROR')"
]
},
{
"cell_type": "markdown",
"metadata": {},
"source": [
"<p style=\"font-size:200%; color:#e56020; background-color:#1d1160;\"><b><i>Reviewer note:</i> Begin PEP8 check cells (delete above when finished)</b></p>"
]
}]
}
182 changes: 182 additions & 0 deletions .github/helpers/pep8_nb_checker.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,182 @@
import argparse
import copy
import json
import numpy as np
import pathlib
import pytz
import re
import subprocess
import sys

from collections import defaultdict
from datetime import datetime as dt

parser = argparse.ArgumentParser()
parser.add_argument('-f', '--file', type=str,
help='The notebook file to be checked')
args = parser.parse_args()

nb_ext = '.ipynb'

try:
nb_file = pathlib.Path(args.file)
if not nb_file.suffix == nb_ext:
raise ValueError(f"file extension must be {nb_ext}")

except Exception as err:
parser.print_help()
raise err

# create a separating line for the script file with unique text, like:
# #################################flake-8-check################################
# (plus a closing newline to avoid W391 at end of file)
identifier = 'flake-8-check'
line_length = 80
fill_0 = (line_length // 2 - 1) - np.floor(len(identifier) / 2).astype(int)
fill_1 = (line_length // 2 - 1) - np.ceil(len(identifier) / 2).astype(int)

separator = '# ' + '#' * fill_0 + identifier + '#' * fill_1 + '\n'
buffer_lines = 3 # sections must end with >2 blank lines to avoid E302

# save relevant file paths
code_file = pathlib.Path(f"{nb_file.stem}_scripted.py")
warn_file = pathlib.Path(f"{nb_file.stem}_pep8.txt")
nb_magic_file = pathlib.Path(".github/helpers/nb_flake8_magic.json")

def check_cell_content(source):
"""Verify that a cell contains any content besides whitespace or newlines"""
for line in source:
if re.search('^(?!(?:\\n)$)', line):
# a "negative lookahead" for any characters in "non-capturing group"
return True

# save code cell contents to a script divided into blocks with the separator
code_cells = []
with open(nb_file) as nf:
og_nb = json.load(nf)

with open(code_file, 'w') as cf:
for i, cl in enumerate(og_nb['cells']):
if (cl['cell_type'] == 'code'
and cl['source']
and check_cell_content(cl['source'])
):
# only check code cells containing actual code; skip blanks
code_cells.append(i)

# check zeroth line for comment on errors to be ignored in this
# cell. if any, generate appropriate "noqa" comment
top_line = cl['source'][0]
noqa_check = re.search('^# flake8-ignore', top_line)
noqa_comment = ('' if noqa_check is None
else ' # noqa' + top_line[noqa_check.end():])

for ln in cl['source']:
# comment out lines with IPython magic commands
line = ln if ln[0] != '%' else '# ' + ln

# insert noqa comment if needed (with care for newline char)
if (noqa_comment and not line.startswith('#')
and line != '\n' and line.endswith('\n')):
line = line[:-1] + noqa_comment
elif (noqa_comment and not line.startswith('#')
and line != '\n'):
line += noqa_comment[:-1]

cf.write(line)
cf.write('\n' * buffer_lines)
cf.write(separator)

# without spawning a shell, run flake8 and save any PEP8 warnings to a new file
with open(warn_file, 'w') as wf:
# flake8's command line options are specified in base repo folder's .flake8
subprocess.run(['flake8', code_file], stdout=wf)

# read in the PEP8 warnings
with open(warn_file) as wf:
warns = wf.readlines()

# if there are none, QUIT while we're ahead
if not warns:
print(f"{nb_file} is clean!")
sys.exit()

# else, read in the script and find the lines that function as cell borders
with open(code_file) as cf:
script = cf.readlines()

borderlines = [j for j, ll in enumerate(script, start=1) # 1-indexed, like file
if re.search(fr"#+{identifier}#+", ll)]

# customize the beginning of each PEP8 warning
pre = dt.now(pytz.timezone("America/New_York")).strftime('%Y-%m-%d %H:%M:%S - INFO - ')
# pre = 'INFO:pycodestyle:'
# pre = ''

# create dict ready to take stderr dicts and append warning messages. the nested
# defaultdict guarantees a first-level key for each cell number needed, and a
# second-level list for appending 'text' strings, and nothing extra (w/o errors!)
stderr_shared = {'name': 'stderr', 'output_type': 'stream'}
nu_output_dict = defaultdict(lambda: defaultdict(list))

# match the warnings' line numbers to the notebook's cells with regex and math
for script_line in warns:
# get this warning's line number in the script
wrn = script_line[re.match(code_file.name, script_line).end():]
script_line_num = int(re.search(r'(?<=:)\d+(?=:)', wrn).group())

# translate it into cell numbers and then to intra-cell line number
code_cell_num = np.searchsorted(borderlines, script_line_num)
all_cell_num = code_cells[code_cell_num]
borderline_num = borderlines[code_cell_num - 1] if code_cell_num > 0 else 0
next_borderline_num = borderlines[code_cell_num]

if script_line_num != next_borderline_num:
line_in_cell = str(script_line_num - borderline_num)
else:
# correct line number for E303 by accounting for buffer added earlier
line_in_cell = str(script_line_num - borderline_num - buffer_lines)

# print(f"--borderline:L{borderlines[code_cell_num - 1]},"
# f"next borderline:L{borderlines[code_cell_num]},"
# f"errorline:L{script_line_num}--")
# print(f"code_cell_num {code_cell_num}, line_in_cell {line_in_cell}, "
# f"all_cell_num {all_cell_num}")

# only keep line/column info and warning from original flake8 text.
# prepend it with the customized string chosen earlier
nu_msg = pre + re.sub(r':\d+(?=:)', line_in_cell, wrn, count=1)

# update the defaultdict
nu_output_dict[all_cell_num].update({'name': 'stderr',
'output_type': 'stream'})
nu_output_dict[all_cell_num]['text'].append(nu_msg)

# use the defaultdict's keys to learn which cells require warnings
cells_to_edit = list(nu_output_dict.keys())
injected_nb = copy.deepcopy(og_nb)

for num, cell in enumerate(injected_nb['cells']):
# clear any cell output, regardless of PEP8 status
if cell.get('execution_count'):
cell['execution_count'] = None
if cell.get('outputs'):
cell['outputs'] = []

# inject PEP8 warnings into cells marked earlier
if num in cells_to_edit:
cell['outputs'] = [nu_output_dict[num]]

# insert cells for enabling interactive PEP8 feedback just above first code cell
# if they aren't already present
with open(nb_magic_file) as nmf:
flake8_magic_cells = json.load(nmf)['cells']

if all([og_nb['cells'][i].get('source') != flake8_magic_cells[0]['source']
for i in code_cells]):
injected_nb['cells'][code_cells[0]:code_cells[0]] = flake8_magic_cells

# save the edited notebook
with open(nb_file, 'w') as file:
json.dump(injected_nb, file, indent=1, ensure_ascii=False)
file.write("\n") # end with new line since json.dump doesn't
44 changes: 44 additions & 0 deletions .github/helpers/tech_review_instructions.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
Hello @%s,

Thank you for submitting these changes to your notebook. Please read on for your technical review instructions.

## Before you begin

The technical review helps ensure that contributed notebooks a) run from top to bottom, b) follow the [PEP8 standards](https://www.python.org/dev/peps/pep-0008/) for Python code readability, and c) conform to the Institute's [style guide](https://github.com/spacetelescope/style-guides/blob/master/guides/jupyter-notebooks.md) for Jupyter Notebooks.

I've pushed the review as a new commit in this pull request. **To view and edit the commit locally, follow these steps:**

```
git checkout %s
git fetch YOUR-REMOTE-FORK
git merge YOUR-REMOTE-FORK/%s
```

_(`YOUR-REMOTE-FORK` is your fork's online copy. It's often `origin`, but if you don't know your name for it, run `git remote -v` and choose the one whose path ends with `%s/%s.git`.)_

From here you can work on your branch as normal. If you have trouble with this step, please let me know before continuing.

---

## Instructions

After updating your local copy of this branch, **please open your notebook and address any warnings or errors you find**.

If you see cells with output like this, it means some of your code doesn't follow the PEP8 standards of code readability:

<img width="574" alt="image" src="https://user-images.githubusercontent.com/12895749/121729210-306c5300-cabc-11eb-90eb-eb494dca53c4.png">

_(In the example above, `INFO - 3:3: E111` means that the text entered on line 3 at index 3 caused the warning "E111". The violation is briefly described at the end of the message.)_

You can test that your edits satisfy the standard by installing `flake8` on the command line with:
```
pip install flake8==3.9.2 pycodestyle_magic
```

Then, restart the notebook and run the following cells:

<img width="580" alt="image" src="https://user-images.githubusercontent.com/12895749/121743209-fd7f8a80-cace-11eb-86a5-90e7b857a8be.png">

After that, edit and re-run cells with warnings until you've fixed all of them. **Please remember to delete the cells shown in the above image before pushing your changes back to this pull request.**

**If you have questions or feedback on specific cells, click the earlier message in this thread from the "review-notebook-app" bot.** There, you can comment on specific cells and view what's changed in the new commit. I may also write comments there. Anything posted there will also be reflected in this pull request's conversational thread.
16 changes: 16 additions & 0 deletions .github/scripts/insert_failure_message.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
# .github/scripts/insert_failure_message.py
# This script inserts a message into the generated HTML to let users know that the notebook currently fails execution
import nbformat as nbf
import sys

notebook_filename = sys.argv[1]

with open(notebook_filename, "r") as f:
nb = nbf.read(f, as_version=4)

new_cell = nbf.v4.new_markdown_cell('<font color="red">This notebook currently fails to execute, use as reference only</font>')
# Insert the cell at the second position in the notebook
nb.cells.insert(1, new_cell)

with open(notebook_filename, "w") as f:
nbf.write(nb, f)
9 changes: 9 additions & 0 deletions .github/workflows/ci_buildondemand.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
name: Manual Execute All Notebooks
on:
workflow_dispatch:

jobs:
ExecuteNotebooks:
uses: spacetelescope/notebook-ci-actions/.github/workflows/ci_scheduled.yml@v3
with:
python-version: ${{ vars.PYTHON_VERSION }}
9 changes: 9 additions & 0 deletions .github/workflows/ci_execute_merge_generate.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
name: Manual Full Archive Execute-Store-Generate HTML
on:
workflow_dispatch:

jobs:
GenerateHTML:
uses: spacetelescope/notebook-ci-actions/.github/workflows/ci_build_merge_manual.yml@v3
with:
python-version: ${{ vars.PYTHON_VERSION }}
Loading

0 comments on commit f19ee4f

Please sign in to comment.