rubin-dp0 / tutorial-notebooks

Tutorial Jupyter Notebooks for Data Preview 0, created and maintained by the Rubin Observatory Community Science Team.
Apache License 2.0
33 stars 17 forks source link

tickets/PREOPS-5247 #203

Closed MelissaGraham closed 1 month ago

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

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

ryanlauastro commented 1 month ago

Feedback on 02a:

Would it make more sense to move up the "Clean up (and save memory) by deleting the job and its results." text earlier in the notebook instead of down in Sec 3.2? This might also help so that the previous searches aren't just being overwritten.

The RA-Dec plots in Sec. 3.3 are displaying the RA in a weird, truncated way "+6.2e1" -- I'm not sure if there's a way to force it to show the values? However, I imagine that displaying them would make the axis label too crowded, so perhaps there is no better way to display this.

In the Sec. 3.4, it says "Use of TOP is preferred, as described in Section 3.1" but Sec. 3.1 doesn't describe the preferred use of "TOP" -- is this meant to be "Sec. 1.4.4" ?

Similarly, in Sec. 3.5, it says "As described in Section 3.1.5, use ORDER BY and TOP together with caution." But there is no Sec. 3.1.5 -- is this meant to be "Sec. 1.4.5" ?

Fig. 4 caption: "right-most edge" -> "left-most edge" (unless the x-axis is meant to be flipped so that RA increases to the left?)

Sec. 4.1.1: "go to data.lsst.cloud and enter the Portal Aspect." should this be "data-int.lsst.cloud" ?


Feedback on 02b:

Sec. 2.4 description: "...to defined colors.." -> "to define colors"


Feedback on 02c:

Sec. 3 description: "Explore the metadata for all images that overlap the target point defined in Section 1.2." -- should this be "Section 1.1" ? Section 1.2 is the DAL error subsection.

Sec. 3.5: "LSST Science Cameras 189 detectors" -> "LSST Science Camera's 189 detectors"

Sec. 3.7: "Recall that in Section 1.2, a function was defined called sregion_to_vertices" -- should this be "Section 1.4" ? Sec. 1.2 is the DAL service error subsection.

Sec. 4.1.2: "Obtain the table dl_results using Datalink and the auth_session defined in Section 1.2." --should this be "Section 1.4" as well?

MelissaGraham commented 1 month ago

Feedback on 02a:

Would it make more sense to move up the "Clean up (and save memory) by deleting the job and its results." text earlier in the notebook instead of down in Sec 3.2? This might also help so that the previous searches aren't just being overwritten.

MLG: Sure, added clean up statements to the ends of the sub-sections of section 2.

The RA-Dec plots in Sec. 3.3 are displaying the RA in a weird, truncated way "+6.2e1" -- I'm not sure if there's a way to force it to show the values? However, I imagine that displaying them would make the axis label too crowded, so perhaps there is no better way to display this.

MLG: There probably is a way to force non-exponential format for the x-axis of Fig 1, but I think it makes sense here (avoid crowding the axis of a small figure) so I'm going to leave it at the matplotlib default.

In the Sec. 3.4, it says "Use of TOP is preferred, as described in Section 3.1" but Sec. 3.1 doesn't describe the preferred use of "TOP" -- is this meant to be "Sec. 1.4.4" ?

MLG: Thank you! This got moved to S.1.4.4. Made the change.

Similarly, in Sec. 3.5, it says "As described in Section 3.1.5, use ORDER BY and TOP together with caution." But there is no Sec. 3.1.5 -- is this meant to be "Sec. 1.4.5" ?

MLG: Yes, changed to S.1.4.5. Then I did a quick search on "Section" to make sure there were no others to fix.

Fig. 4 caption: "right-most edge" -> "left-most edge" (unless the x-axis is meant to be flipped so that RA increases to the left?)

MLG: Fixed thank you! just mixing up my L and R again...

Sec. 4.1.1: "go to data.lsst.cloud and enter the Portal Aspect." should this be "data-int.lsst.cloud" ?

MLG: No, users do not have accounts at data-int, only staff.

Feedback on 02b:

Sec. 2.4 description: "...to defined colors.." -> "to define colors"

MLG: fixed

Feedback on 02c:

Sec. 3 description: "Explore the metadata for all images that overlap the target point defined in Section 1.2." -- should this be "Section 1.1" ? Section 1.2 is the DAL error subsection.

MLG: The target point is defined in S.1.4 so updated that.

Sec. 3.5: "LSST Science Cameras 189 detectors" -> "LSST Science Camera's 189 detectors"

MLG: Fixed.

Sec. 3.7: "Recall that in Section 1.2, a function was defined called sregion_to_vertices" -- should this be "Section 1.4" ? Sec. 1.2 is the DAL service error subsection.

MLG: Fixed.

Sec. 4.1.2: "Obtain the table dl_results using Datalink and the auth_session defined in Section 1.2." --should this be "Section 1.4" as well?

MLG: Yup! Fixed.