Skip to content

Pull Request / Code review#3

Open
jvntra wants to merge 5 commits intoPhys767-Spring17:masterfrom
jvntra:master
Open

Pull Request / Code review#3
jvntra wants to merge 5 commits intoPhys767-Spring17:masterfrom
jvntra:master

Conversation

@jvntra
Copy link
Copy Markdown
Collaborator

@jvntra jvntra commented May 24, 2017

I can't find Victoria to initialize the code review... Only options listed are Kelle Cruz and Eileen Gonzalez.

@kelle kelle requested a review from vditomasso May 24, 2017 20:07
Comment thread ewidth.py
# Jean-paul Ventura
# January 12, 2017
# Modified: February 20th, 2016
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

It's not clear what the significance of these dates are and I'm not sure that you need them because GitHub tracks when you change things.

Comment thread ewidth.py
This function measures the equivalent width of spectral emission/absorption features.

====================
Function parameters:
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Is there a reason this is "Function parameters" and not "Arguments"?

Comment thread ewidth.py
hdu = fits.open(filename)

wavelength2 - wavelength value where continuum region to the left of the spectral feature ends.
# Define wavelength and flux arrays by accessing .fits sloan data
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Does this code only work for sloan data? If so, include that in the description of the function in your docstring/readme

Comment thread ewidth.py
fitl = np.array(m*lmbda + c)

# Normalize the flux array by dividing it by the fitline.
nflux = flux/fitline
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Where is the variable "fitline" defined?

Comment thread ewidth.py
# Sum continuosly over the normalized flux array and assign result to a variable.
eqwi = sum(1-nflux[ewindx])*dlmbda

# Calculate the uncertainty in the equivalent with measurement
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

width*

Comment thread ewidth.py

"""
# assign the number of monte carlo iterations to variable 'size'.
size = 3000
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

"iter" might be a better variable name than "size"

Comment thread ewidth.py
#Calculate line height of spectral emission feature
def line_height(filename,wavelength1,wavelength2,wavelength3,wavelength4):
"""
Tnis function calculates the emission line feature height as the vertical-distance between the
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This*

Comment thread ewidth.py
# Isolate the wavelength subarrays pertaining to both of the wings of the line feature
x1 = lmbda[coindx]

#Isolate the normalized flux subarrays of the line features wings.
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

feature's*

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.

2 participants