search

spacetelescope / dat_pyinthesky

Notebooks for "notebook-driven development" for the Data Analysis Tools efforts
https://dat-pyinthesky.readthedocs.io/en/latest/
8 stars 44 forks source link

Migrate WFC3Library calwf3 recalibration notebook #188

Closed kecnry closed 2 years ago

kecnry commented 2 years ago

DO NOT MERGE: This PR is to demonstrate the technical review process for creating a new or updating an existing notebook.

review-notebook-app[bot] commented 2 years ago

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.

Powered by ReviewNB

ojustino commented 2 years ago

Hi @kecnry,

Thank you for submitting this notebook. Please read on for the technical review.

Before you begin

The technical review helps ensure that contributed notebooks a) run from top to bottom, b) follow the PEP8 standards for Python code readability, and c) conform to the Institute's style guide 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 wfc3library-calwf3recal
git fetch YOUR-REMOTE-FORK
git merge YOUR-REMOTE-FORK/wfc3library-calwf3recal

_(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 kecnry/dat_pyinthesky.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:

image

(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 under the imports:

image

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.

The three-point review (*action required*)

  1. ✅ Notebook execution: I was able to run the notebook from top to bottom.

  2. ⚠️ Code style: There are a good number of PEP8 violations.

    • Please follow the advice in the "Instructions" section above to fix them.

  3. ⚠️ Notebook style

    • I will leave a comment on a cell that could use a change.
ojustino commented 2 years ago

I will close this pull request now that the HST tutorial has passed. Thanks again for playing along, @kecnry.