SP-2645: added 301 notebook on image display for faint features#158
SP-2645: added 301 notebook on image display for faint features#158garrethmartin wants to merge 3 commits into
Conversation
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
| @@ -0,0 +1,543 @@ | |||
| { | |||
There was a problem hiding this comment.
Question CST probably has not encountered before: In RTN-045 do we adopt a specific english spelling (British vs American)? Visualise vs visualize, and characterise, etc, but there's many other words like catalog (perhaps we should ask Melissa what to do here). If there's no policy about this then please disregard this comment!
Update from Melissa: we dont' really care about this, except to appease the spellchecker. So perhaps just look through all markdowns prior to merging to check for any other flagged spelling. I will flag here where I noticed the spellchecker flagged in the notebook aspect:
visualise -> visualize, characterise -> characterize
Also regarding lsb-202 and lsb-301, should these be capitalized since its an abbreviation? Maybe its better to list the actual notebook number instead?
Reply via ReviewNB
| @@ -0,0 +1,543 @@ | |||
| { | |||
There was a problem hiding this comment.
Perhaps move the text starting with "two functions are used..." to the next markdown right before function definition
artefact -> artifact
Reply via ReviewNB
| @@ -0,0 +1,543 @@ | |||
| { | |||
There was a problem hiding this comment.
| @@ -0,0 +1,543 @@ | |||
| { | |||
There was a problem hiding this comment.
Functions should have the inputs and outputs defined in the commented description e.g.:
def gauss(x, a, x0, sigma):
"""Evaluate a 1D Gaussian function.
Parameters
----------
x : array_like
Input values where the Gaussian is evaluated.
a : float
Amplitude of the Gaussian peak.
x0 : float
Mean (center) of the Gaussian.
sigma : float
Standard deviation (width) of the Gaussian.
Returns
-------
y : array_like
Values of the Gaussian function evaluated at x.
"""
return a * np.exp(-(x - x0)**2 / (2 * sigma**2))
Reply via ReviewNB
| @@ -0,0 +1,543 @@ | |||
| { | |||
There was a problem hiding this comment.
| @@ -0,0 +1,462 @@ | |||
| { | |||
There was a problem hiding this comment.
Suggest splitting this cell into individual functions or group of related functions (separate from parameter definitions) so that they each get a descriptive markdown to help understand what they do
Also inputs and outputs should be defined in the functions (see example in review in 312.1)
Reply via ReviewNB
| @@ -0,0 +1,462 @@ | |||
| { | |||
There was a problem hiding this comment.
Suggest splitting this code block and inserting some narrative markdown at each stage to help the reader
Reply via ReviewNB
| @@ -0,0 +1,462 @@ | |||
| { | |||
There was a problem hiding this comment.
| @@ -0,0 +1,462 @@ | |||
| { | |||
There was a problem hiding this comment.
Are there any tips to get a good science measurement for a first-time user? For example avoid background objects or exclude flux from the host or something?
Reply via ReviewNB
| @@ -0,0 +1,462 @@ | |||
| { | |||
There was a problem hiding this comment.
Sections 4-5 are quite fun! Nice
How does one accurately calculate the error on photometry for a weird irregular shape? Is it just the average surface brightness and then scaled by number of pixels in painted region or something?
Reply via ReviewNB
|
Nice! I left lots of comments and suggestions in the 3 notebooks. Please let me know if you have any questions about any of it! Nice job. |
christinawilliams
left a comment
christinawilliams
left a comment
There was a problem hiding this comment.
Left some comments and feedback in the 3 notebooks, please let me know if there are any questions! Nice job!
3 participants
SP-2645: new tutorial on image display