Skip to content
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

Add ATLAS as a builtin #336

Merged
merged 3 commits into from
Jan 26, 2022
Merged

Add ATLAS as a builtin #336

merged 3 commits into from
Jan 26, 2022

Conversation

mcoughlin
Copy link
Contributor

Given that ATLAS provides a very popular forced photometry service used by transient folks, having its filters would seem appropriate.

It looks like someone has to add the SVO files to http://sncosmo.github.io/data, but otherwise seems straightforward.

@mcoughlin
Copy link
Contributor Author

@benjaminrose I fixed the line too long I hope.

@benjaminrose
Copy link
Member

It looks like you are missing a closing ) in the last commit. But more importantly is that download failed. Are the filters defined somewhere, or do they need to be uploaded to sncosmo.github.io & maintained by SNCosmo?

@mcoughlin
Copy link
Contributor Author

They are here: http://svo2.cab.inta-csic.es/svo/theory/fps/index.php?id=Misc/Atlas.orange&&mode=browse&gname=Misc&gname2=Atlas

So if they can be pulled over to sncosmo.github.io would be great.

@benjaminrose
Copy link
Member

I think I added the needed redirects to the website, sncosmo/sncosmo.github.io@d4c4dca.

@mcoughlin
Copy link
Contributor Author

@benjaminrose And I think I got that parentheses.

@mcoughlin
Copy link
Contributor Author

@benjaminrose maybe this test just needs to be retriggered?

@benjaminrose
Copy link
Member

New builtins need an update to both the code and the website. I tried to update the website in a slightly different, but I hoped more sustainable, way. It seems to not have worked. I’ll update it the old way soon and then the last test should pass.

Once we get that passing, we should be able to merge this PR.

@mcoughlin
Copy link
Contributor Author

Thanks @benjaminrose !

@mcoughlin
Copy link
Contributor Author

@benjaminrose Is there anything I can to support getting the tests to pass?

@kboone
Copy link
Member

kboone commented Jan 26, 2022

It looks like the last failure was a stochastic one and unrelated to this PR. I'm rerunning the tests now.

@kboone
Copy link
Member

kboone commented Jan 26, 2022

LGTM

@kboone kboone merged commit f2933b2 into sncosmo:master Jan 26, 2022
@mcoughlin
Copy link
Contributor Author

Thanks @benjaminrose and @kboone !

@benjaminrose
Copy link
Member

Thanks @kboone for rerunning it. And I am glad that we can use redirects.json rather than hosting the throughputs ourselves.

@mcoughlin, sorry for taking a while to get back to this PR. Glad it got the attention it needed.

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.

3 participants