AstarVienna / ScopeSim_Templates

GNU General Public License v3.0
2 stars 4 forks source link

Fix scaling of spectrum #74

Closed hugobuddel closed 9 months ago

hugobuddel commented 9 months ago

Fixes #73

codecov[bot] commented 9 months ago

Codecov Report

Attention: 8 lines in your changes are missing coverage. Please review.

Comparison is base (25c262f) 79.18% compared to head (13deda0) 79.58%.

Files Patch % Lines
scopesim_templates/tests/test_misc/test_misc.py 87.87% 8 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## dev_master #74 +/- ## ============================================== + Coverage 79.18% 79.58% +0.40% ============================================== Files 44 44 Lines 1970 2048 +78 ============================================== + Hits 1560 1630 +70 - Misses 410 418 +8 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

hugobuddel commented 9 months ago

Perhaps we should fix the bug that requires synphot templates to require 4 or more wavelength points instead of these kludgey workarounds.

The tests I added here do not give any flux when only 2 or 3 points are used. I think it might be a problem with determining overlaps. Not sure.

hugobuddel commented 9 months ago

Thanks for catching that possible off-by-0.5 error. The source is still far from the center in the simulated images though... The peak value (so ~center) seems to be at 2123,2123 instead of 2048,2048.

Nevertheless, the exact position of the source doesn't really matter here, as it is a test for the photometry, not the astrometry. The source should just not be entirely at the corner. So I decided to not investigate that offset further.

I created #75 to fix the problem of requiring 4 or more points for a synphot spectrum (or all spectra?), as it is also essentially independent of this problem. Or well, it did make scaling of the spectra in source_from_imagehdu broken, but I 'fixed' that by simply using enough points.

So I prefer to merge this first, and since you said 'everything else is good', I'll do just that :-).

Also, I turned the figures off by default, oops.