SP-2072: add new 303-series tutorial on color-color selections#155
Conversation
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
| @@ -0,0 +1,985 @@ | |||
| { | |||
There was a problem hiding this comment.
There was a problem hiding this comment.
Added, thanks!
| @@ -0,0 +1,985 @@ | |||
| { | |||
There was a problem hiding this comment.
There was a problem hiding this comment.
Whoops thanks for catching that. I also updated the other sections to be consistently hash-tagged.
| @@ -0,0 +1,985 @@ | |||
| { | |||
There was a problem hiding this comment.
| @@ -0,0 +1,985 @@ | |||
| { | |||
There was a problem hiding this comment.
"!.2." is inconsistent with the rest of the section numbering, which don't have trailing period (i.e. "1.2")
Reply via ReviewNB
There was a problem hiding this comment.
Whoops I always get confused about this. It looks like the policy is to include the trailing period so I have updated all the other sections that are missing one.
| @@ -0,0 +1,985 @@ | |||
| { | |||
There was a problem hiding this comment.
Line #14. f"CIRCLE('ICRS', {target_ra}, {target_dec}, 0.5)) = 1"
the markdown says 0.2 degrees, but the query of for 0.5 degrees
Reply via ReviewNB
There was a problem hiding this comment.
Good catch, I fixed. Search radius 0.5 was needed to maximize the matches.
| @@ -0,0 +1,985 @@ | |||
| { | |||
There was a problem hiding this comment.
There was a problem hiding this comment.
Fixed (and fixed caption for fig 3 as welll)
| @@ -0,0 +1,985 @@ | |||
| { | |||
There was a problem hiding this comment.
| @@ -0,0 +1,985 @@ | |||
| { | |||
There was a problem hiding this comment.
Line #22. query = f"""
shadows the previous query variable in Section 2.. consider changing
Reply via ReviewNB
There was a problem hiding this comment.
I updated query to "cutout_query" thanks for the suggestion
| @@ -0,0 +1,985 @@ | |||
| { | |||
There was a problem hiding this comment.
Line #30. coadds = tap_service.search(query).to_table()
Tutorial guidelines recommend that all TAP queries to be asynchronous
Reply via ReviewNB
There was a problem hiding this comment.
I have updated to asynchronous query.
| @@ -0,0 +1,985 @@ | |||
| { | |||
There was a problem hiding this comment.
Line #2. failed_spec_z = matched_v_all['zsp'][failed_lbg_indices]
I think the filtering contains a bug, np.where returns indices into the already filtered array. so i think it should instead just be:
failed_spec_z = selected_spec_z[selected_spec_z < 3.2]
Reply via ReviewNB
There was a problem hiding this comment.
You're right, my mistake, thanks for catching.
garrethmartin
left a comment
There was a problem hiding this comment.
Approved. See comments for some minor requested changes.
b8ea2c7 to
16938f1
Compare
Adding new DP1 galaxies series 303 notebook on color color selections