search

spacetelescope / stsynphot_refactor

Synthetic photometry using Astropy for HST and JWST
http://stsynphot.readthedocs.io/en/latest/
BSD 3-Clause "New" or "Revised" License
14 stars 10 forks source link

TST: Test with new lookup tables #147

Closed pllim closed 3 years ago

pllim commented 3 years ago

Description

This pull request is to test against new tables, as requested.

Close #146

@sean-lockwood , I don't know if simply looking at the results here is good enough for you, or you actually want these files to be part of the CI test suite. When run locally, I see failures but I think they are the usual numerical changes as INS continually updates the throughputs. Nothing crashed spectacularly. But also note that these are unit tests; they are not exhaustive but hopefully HST ETC test suite on your end will smoke out the rest. Hope you can clarify your acceptance criteria here. Thanks!

From /grp/redcat/trds/mtab/:

github-actions[bot] commented 3 years ago

👋 Thank you for your draft pull request! Do you know that you can use [ci skip] or [skip ci] in your commit messages to skip running continuous integration tests until you are ready?

sean-lockwood commented 3 years ago

I was mostly concerned about the formatting change to the TMG causing errors. Failed numerical differences are to be expected due to throughput changes in HST instruments.

pllim commented 3 years ago

Luckily new column didn't break the parser:

https://github.com/spacetelescope/stsynphot_refactor/blob/5a2fa008a4f2d48f768e3a3abcdc1220a313f0eb/stsynphot/stio.py#L309

And seems completely ignored by the class:

https://github.com/spacetelescope/stsynphot_refactor/blob/5a2fa008a4f2d48f768e3a3abcdc1220a313f0eb/stsynphot/tables.py#L18

pllim commented 3 years ago

If you are satisfied by the results here and okay with closing this PR without merge, then my job here is done. Please let me know. Thanks!

sean-lockwood commented 3 years ago

Thanks, @pllim ! I think your summary satisfies my concern.