-
Notifications
You must be signed in to change notification settings - Fork 20
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
Bugifx ql timeseries units #118
Bugifx ql timeseries units #118
Conversation
b500ee7
to
934e0b3
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #118 +/- ##
==========================================
+ Coverage 69.52% 72.53% +3.00%
==========================================
Files 31 31
Lines 1887 1857 -30
==========================================
+ Hits 1312 1347 +35
+ Misses 575 510 -65 ☔ View full report in Codecov by Sentry. |
* Update plot code to use more of sunpy TimeSeries functions * Add columns keyword with useful defaults
934e0b3
to
cccf11f
Compare
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.
Where's the key change that actually scales the data by time correctly?
data["variance"] = data["variance"].reshape(-1) / (dE * data["timedel"]) | ||
data["variance"] = data["variance"].reshape(-1) * u.ct / (delta_energy * data["timedel"].to(u.s)) |
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.
I'm actually not totally sure what units should be here TBH or if we should be correcting for live time.
Co-authored-by: DanRyanIrish <ryand5@tcd.ie>
OK. Well, as long as the tests pass and the plots look correct, I'm happy to approve this PR. |
stixpy/timeseries/quicklook.py
Outdated
r""" | ||
Quicklook X-ray time series from the background detector. | ||
|
||
Background monitoring is done in such way that counts from background detector are summed in five specified energy |
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.
again I'd say maybe just mention that this is a QL data product and the file
Co-authored-by: Laura Hayes <lauraannhayes@gmail.com>
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.
once tests pass, all good :)
c1305ae
to
60298dd
Compare
Urgh - I don't understand the windows test fail on HK it happening at download time which is done in sunpy and the same thing works for the QL 😭 |
Closes #104