Add tests, rearrange GLOWS L2, fix exposure times#2726
Open
maxinelasp wants to merge 2 commits intoIMAP-Science-Operations-Center:devfrom
Open
Add tests, rearrange GLOWS L2, fix exposure times#2726maxinelasp wants to merge 2 commits intoIMAP-Science-Operations-Center:devfrom
maxinelasp wants to merge 2 commits intoIMAP-Science-Operations-Center:devfrom
Conversation
tech3371
approved these changes
Feb 19, 2026
| # A good time is where all the active flags are equal to one. | ||
| # Here, we mask the active indices using active_flags, and then return the times | ||
| # where all the active indices == 1. | ||
| good_times = np.where(np.all(flags[:, active_flags == 1] == 1, axis=1))[0] |
Contributor
There was a problem hiding this comment.
I don't know enough details here but feel like it could be improved. But I don't have suggestions yet.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Change Summary
Overview
Moves GLOWS L2 Histogram calculation into the dataclass to match other levels, fixes the L2 exposure time calculation, and adds additional unit tests.
A lot of these changes are just for moving calculations from glows_l2.py to glows_l2_data.py. Look at the specific updates in the file changes section for actual changes. The main ones are around exposure_times.
File changes
glows_l2_data.py: Fixes exposure time calculation inDailyLightcurve— divides by number of bins (not hardcoded 3600) and accumulates uniformly across good-time epochs rather than per-bin. Removes the now-incorrect
calculate_exposure_timesstatic method. Fixes an in-place mutation bug incalculate_histogram_sums. CompletesHistogramL2.__init__with all L2 averages and std devs computed over good-time blocks only, with correct shapes for scalar (keepdims=True) and vector ([np.newaxis, :]) fields. Convertsreturn_good_timesto a@staticmethod.glows_l2.py: Removes logic that was moved intoHistogramL2.__init__.test_glows_l2_data.py: New test file coveringDailyLightcurve(photon flux, flux uncertainty, zero-exposure bins, number of bins) andHistogramL2.return_good_times.test_glows_l2.py: Updatestest_generate_l2assertions to match corrected output values; removestest_exposure_timeswhich tested the removedcalculate_exposure_timesmethod.Testing
test_glows_l2_data.pytest_glows_l2.pyupdated to reflect corrected calculation behaviorTests were generated with Claude but checked by hand. The averaging steps in init were also done with AI. Nothing else was completed with AI, although it was checked against the validation scripts with Claude.
Completes tickets: #2367
Also fixes an un-ticketed bug with exposure time calculation