Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NIRSpec FS Pipeline Demo Notebook #26

Open
wants to merge 15 commits into
base: main
Choose a base branch
from

Conversation

kglidic
Copy link

@kglidic kglidic commented Nov 4, 2024

This notebook checklist has been made available to us by the 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

  • 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 (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 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. This could be

    • Alt text (an alt property)
    • Empty alt text 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 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 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 (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.

Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@drlaw1558
Copy link
Collaborator

Tagging @rizeladiaz

@kglidic
Copy link
Author

kglidic commented Nov 5, 2024

I have updated the notebook to use the latest pipeline version 1.16.0 and correct CRDS context.

@rizeladiaz
Copy link
Collaborator

Started reviewing this notebook

@sosey sosey requested a review from rizeladiaz November 21, 2024 13:57
@rizeladiaz
Copy link
Collaborator

I completed the review of this notebook. Just waiting for @kglidic 's approval.

Updates from technical review FS
@kglidic
Copy link
Author

kglidic commented Jan 9, 2025

Thank you, @rizeladiaz, for thoroughly reviewing this notebook. I’ve incorporated your feedback and made a few changes:

  1. I addressed formatting and spelling issues and removed repetitive timestamps.
  2. Previously, the notebook contained some code that compared the results to those of MAST products. I’ve removed any remaining calls to this (but it remains a parameter in the helper function).
  3. I added print statements to compare the default CRDS context associated with the build with the context actively set in the notebook as a check.
  4. Examples for Creating Association (ASN) Files: I added a helper function, write_asn, with examples for manually creating ASN files for spec2 and spec3. This function was part of an earlier version of the notebook I worked on, but I removed it after discussions about how some observations might have more complex ASN files (which my current function may not be able to handle). For example, the current function can handle spectral dithers and dedicated background exposures when creating spec2 ASN files. But, it would likely not handle spatial dithers properly. So, by default, the notebook is set to download the ASN files from MAST, but if the user chooses to create their own, they have an example. If we wanted the notebook to handle more complex cases, I’d have to think of the best way to do so and make edits. 

If there is no issues/concerns with the changes made above, I think the notebook should be good to go.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants