-
Notifications
You must be signed in to change notification settings - Fork 35
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
Update the regression test to METAL observed/AST dataset #612
Conversation
Codecov Report
@@ Coverage Diff @@
## master #612 +/- ##
==========================================
- Coverage 47.25% 43.01% -4.24%
==========================================
Files 94 94
Lines 9079 9123 +44
==========================================
- Hits 4290 3924 -366
- Misses 4789 5199 +410
Continue to review full report at Codecov.
|
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.
Looks good! I like the new file organizational system for the tests (both in the new class and in the folders online).
One comment about plot_indiv_fit
: the filters are still hard-coded in, so it'll only work for a particular survey. I'd imagine it would be straightforward to extract them from the stats file, and maybe use pysynphot or something to get the wavelength from the corresponding filter curve. But that may be outside the scope of this PR :).
For the tests, perhaps getting everything consistently regenerated would be a good topic for an upcoming hack day??? 😄
Good point! I've been wondering why plot_indiv_fit has been looking odd for the metal results. Will see what I can do about this in the next PR (not this one). |
Good idea. I've been thinking we should have one soon. I'll setup a doodle poll. |
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.
Thanks for making the changes!
The current regression tests are based on the beast-examples/phat_small dataset (obs/AST results). The details of the phat_small dataset are lost in the mists of time. The phat_small dataset is old enough that it does not have the full information that we now use in the BEAST (e.g., the ASTs only report mags for the recovered measurements, instead of fluxes that all more recent AST results provide).
This new dataset is based on a subset of a METAL field.
A number of regression tests are skipped as they need updated cache files either for phat or metal or both. See #598.
Includes minor updates to plot_indiv_fit.py.
Includes various updates to close files left open and remove some deprecation warnings. This cleans up the test output making it easier to see problems.
Partially addresses #567.
Closes #325.