New WFC3 Notebook: uvis_amp_equalization#481
Conversation
annierose3
requested review from
annapayne,
cshanahan1 and
gibsongreen
as code owners
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
|
This notebook is still under WFC3 team review! I will update to "ready for review" when it is ready for DMD review. Thank you! :) |
|
@annierose3 Thank you for your patience! I've finished my first review and am ready to pass it back to you for edits. Make sure you create a Great work on this notebook!! It's going to be a great addition to our repo! |
| "cell_type": "markdown", | ||
| "metadata": {}, | ||
| "source": [ | ||
| "<a id='top'></a>\n", |
There was a problem hiding this comment.
Add the new ST logo to the notebook.
Add:  after <a id='top'></a>. Be sure there is a blank line above and below the logo so everything else renders and works properly
| "\n", | ||
| "\n", | ||
| "\n", | ||
| "The amp-offset correction workflow in this notebook follows the method from [Sunnquist (2019)](https://github.com/bsunnquist/uvis-skydarks) and uses much of the same code. An overview of the steps we will take to achieve this are as follows:\n", |
There was a problem hiding this comment.
Grammar: An overview of the steps we will take to achieve this is as follows
| "4. Multiply these offsets by the flat field to account for sensitivity variations.\n", | ||
| "5. Subtract the resulting amp-by-amp corrections from the original `FLC`/`FLT` data.\n", | ||
| "\n", | ||
| "If necessary, users should then re-drizzle their data using the corrected images to create new `DRZ`/`DRC` files after completing the above steps. \n", |
There was a problem hiding this comment.
Move the info from Learning Goals into the Introduction.
The learning goals section should be brief and provide a quick bulleted list of what you do in the notebook. E.g., download data from MAST, build a segmentation map, measure the quadrants, apply flat and equalize; Or something along those lines. In my opinion, the overview of the steps should go in the introduction.
| "cell_type": "markdown", | ||
| "metadata": {}, | ||
| "source": [ | ||
| "---\n", |
There was a problem hiding this comment.
Unless you really like them, consider removing all the horizontal lines between the sections. Our WFC3 notebooks don't use this convention. You can, however, add this line under the title if you want. Some of our notebooks do that.
| "---\n", | ||
| "## 1. Introduction <a id='intro'></a>\n", | ||
| "\n", | ||
| "The WFC3/UVIS detector is made up of four amplifiers (A, B, C, D), each covering one quadrant of the 4Kx4K pixel field. Even after the standard `calwf3` pipeline reduction, small offsets to the bias level can persist between quadrants. These offsets are relatively constant across each quadrant, and sometimes show up as visible to the eye with some amps brighter than others.\n", |
There was a problem hiding this comment.
Suggested rewording: "The WFC3/UVIS detector has four amplifiers..."
There is a double space between "as" and "visible".
Consider adding a comma between "eye" and "with"
| "outputs": [], | ||
| "source": [ | ||
| "for fname in files:\n", | ||
| " example_file = fname\n", |
There was a problem hiding this comment.
I don't see any reason to alias/copy fname to a new variable example_file. For clarity, please remove example_file and just use fname.
| " data[segmap > 0] = np.nan\n", | ||
| "\n", | ||
| " # Split into left (amp A/C) and right (amp B/D) halves\n", | ||
| " left, right = np.split(data, 2, axis=1)\n", |
There was a problem hiding this comment.
I didn't know np.split was a thing. That's so convenient/efficient; good work!
| "metadata": {}, | ||
| "source": [ | ||
| "---\n", | ||
| "## 7. Step 3 — Compute the Per-Amp Offsets <a id='offsets'></a>\n", |
There was a problem hiding this comment.
This section could benefit from being a little more explicit. You say both overall mean and overall median. The overall mean is the mean of the per-amp medians
| "source": [ | ||
| "print('Amp medians before and after equalization\\n')\n", | ||
| "print(f'{\"File\":<30} {\"State\":<8} {\"A\":>8} {\"B\":>8} {\"C\":>8} {\"D\":>8} {\"Spread\":>8}')\n", | ||
| "print('-' * 85)\n", |
There was a problem hiding this comment.
bump up 85 to ~90 so that the underline extends further
| "# The following lines of code find and download the MDRIZTAB reference file which can be used as drizzle parameters\n", | ||
| "mdz = fits.getval(eq_files[0], 'MDRIZTAB', ext=0).split('$')[1]\n", | ||
| "print('Searching for the MDRIZTAB file:', mdz)\n", | ||
| "!crds sync --hst --files {mdz} --output-dir {os.environ['iref']}" |
There was a problem hiding this comment.
same as above; use os.system instead of !
2 participants
This notebook checklist has been made available to us by the Notebooks For All team.
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
or # in markdown).
The Rest of the Cells
Text
Code
Images (None in this notebook)
[N/A ] All images (jpg, png, svgs) have an image description. This could be
[N/A] Any text present in images exists in a text form outside of the image (this can be alt text, captions, or surrounding text.)
Visualizations (still need to verify this notebook satisfies these reqs.)
All visualizations have an image description. Review the previous section, Images, for more information on how to add it.
Visualization descriptions include
All visualizations and their parts have enough color contrast (color contrast checker) 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. 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.