Skip to content

Conversation

@MelissaGraham
Copy link
Contributor

No description provided.

@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

Copy link
Collaborator

@jeffcarlin jeffcarlin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall looks good -- thanks for the improvements! Some minor comments below (the choice about the subthresh section is left to you).

ReviewNB won't let me submit my comments, so here they are in text form:

Sec. 3.1 (at "Query the butler for overlapping visit images..."): butler.query_datasets will accept a "limit=1" kwarg. Would it be better to use that to ensure that it only returns one reference?

Sec. 4: (1) I feel like if our goal is to keep tutorials "bite-sized", then this (subthreshold) section should be separated into an additional tutorial (which maybe should include the actual detection and identification of the subthreshold sources, instead of just providing a table of unknown origin).

Sec. 4: (2) It is a good point that we should remind users that they can define the RA, Dec coords for forced photometry (i.e., they don't have to load a DP1 table). Maybe if we split off the subthreshold section, then we could just add a brief section that makes an Astropy table with a handful of arbitrary RA, Dec entries and runs forced photometry plus display on an image?

Re. the "subthresh" catalog: "rmage" is an unclear name for a column -- maybe "rmag_error" or "err_rmag"?

Fig. 3 (Sec. 4.3): The inverted axes are confusing to me. I suggest removing that. Also, this might just be a me thing, but I strongly prefer comparison plots like this to show the difference on the y-axis instead of a 1:1 comparison. To me, that conveys more information at a quick glance.

@MelissaGraham
Copy link
Contributor Author

Issue 1: Query returns are ambiguous.
Fix: Added order_by and limits to queries, so that until a user changes the inputs the tutorial will always run with the same images.

Issue 2: The NB too long now.
Fix: Section 3 is a repeat of Section 2, but instead of removing it, it's just been reduced. Section 4 has also been reduced as much as possible. A whole separate NB to repeat forced photometry with a user table is not needed.

Issue 3: The description of the sub-threshold input table was insufficient.
Fix: Instead of a single sentence, a longer description of how this table was generated now exists at the start of Section 4.

Issue 4: Forced photometry at random locations is not demonstrated.
Not fixed: This is on purpose, mostly because no comparison magnitudes would exist, but also to keep the NB short and because it's trivial to adapt Section 4 to do random coords.

Issue 5: "rmage" is not a good column name.
Fix: Updated to "r_mag_err".

Issue 6: Axes are too confusing.
Fix: Changed the final comparison plot to be diff vs mag.

@MelissaGraham MelissaGraham merged commit 2ead6d5 into main Dec 15, 2025
2 checks passed
@MelissaGraham MelissaGraham deleted the tickets/SP-2826 branch December 15, 2025 21:08
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