Skip to content

Update SNIDsn.py#9

Open
sk7372 wants to merge 17 commits intometal-sn:masterfrom
sk7372:may2020_test
Open

Update SNIDsn.py#9
sk7372 wants to merge 17 commits intometal-sn:masterfrom
sk7372:may2020_test

Conversation

@sk7372
Copy link
Copy Markdown

@sk7372 sk7372 commented May 28, 2020

added if/else statements to account for when data files only have phases, wavelengths, and spectra.

Comment thread code/SNIDsn.py Outdated
continuum[ind - 1] = np.array([float(x) for x in cont_line])
self.continuum = continuum
return
if len(header_lines) == 10: #if file matches tutorial data format
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 is hard for users to understand. Use '.lnw' for SNID templates. If suffix of the user specified file isn't lnw, then check to see if the file has two columns, one for wavelength and one for flux. If it does, make a SNIDsn object, otherwise, raise an assert.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

look up Python assertions.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Maybe allow ascii files with at least 2 columns. Require a header line first that names the columns.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

your new function should have a phaseType argument for the user to specify whether phases are defined relative to max or not.

Comment thread code/SNIDsn.py
@@ -410,59 +410,86 @@ def loadSNIDlnw(self, lnwfile):
with open(lnwfile) as lnw:
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Write a new function within this class: loadSN(self, file, phaseType)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Add other arguments as necessary.

Copy link
Copy Markdown

@marxwillia marxwillia left a comment

Choose a reason for hiding this comment

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

It's very important to include a test with your PR to show that your new code works.

@marxwillia
Copy link
Copy Markdown

Use git checkout SNIDsn.py to throw away your changes.

Comment thread code/SNIDsn.py Outdated
assert file[-3:] != 'lnw' #file cannot be .lnw
self.phaseType = phaseType

all_data = np.loadtxt(file, skiprows = 1)
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 is assuming that header line is not marked with a '#' i think?

Comment thread code/SNIDsn.py

all_data = np.loadtxt(file, skiprows = 1)

header_line = all_data[0]
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Didn't you skip the header line in the line above?

Comment thread code/SNIDsn.py Outdated
all_data = np.loadtxt(file, skiprows = 1)

header_line = all_data[0]
phases = header_line[1:len(header_line)]
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

phases = header_line[1:] works too.

Comment thread code/SNIDsn.py Outdated
Comment on lines +497 to +501
for row in all_data:
wvl.append(row[0])
wvl.pop(0)
wvl = np.array(wvl)
self.wavelengths = wvl
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

np.loadtxt already gives you an ndarray with array slicing ability.

self.wavelengths = all_data[:, 0] grabs entries in every row of the 0th column.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

self.wavelengths = all_data[1:, 0] skips the first row

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

self.wavelengths = all_data[[1,2,3,4], 0] which gives you the wavelengths in rows 1,2,3,4

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