-
Notifications
You must be signed in to change notification settings - Fork 5
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
U/jrbogart/snana support #70
Conversation
Include commit recently merged to main which github thought might conflict (but doesn't really)
2. Use "sncosmo" subdirectory of skycatalog root rather than "reorg" in __main__ tests because "reorg" doesn't have files for all needed healpixels
skyCatalogs - pass path of snana sed file to SnanaCollection snana_object - move caching of sed file in SnanaCollection to separate routine, not __init__ Add temporary routine SnanaObject._read_nearest_sed as a stopgap until real interpolation code can be written
Note also catalog_creator was modified in previous commit to use extinction column utilities
bebacd3
to
5fbbf92
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be good to add some test code for all of these changes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I found one potentially fatal error, i.e., the undefined last_ix
variable. Once that's fixed, I think it's ok to merge. I'd also suggest running flake8 or ruff on all of the source code since there are other undefined variables and similar issues elsewhere.
skycatalogs/objects/snana_object.py
Outdated
if index == 0: | ||
mjd_ix_l = mjd_ix_u = 0 | ||
elif index == len(mjds): | ||
mjd_ix_l = mjd_ix_u = last_ix |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
last_ix
is undefined. Looks like this should be
mjd_ix_l = mjd_ix_u = index - 1
Provides standard skyCatalogs API for SNANA-generated catalogs.
Expect flux calculation to be improved in the future.