fix_issue#34 #38
fix_issue#34 #38
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #38 +/- ##
==========================================
+ Coverage 95.24% 95.90% +0.65%
==========================================
Files 3 4 +1
Lines 505 537 +32
==========================================
+ Hits 481 515 +34
+ Misses 24 22 -2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Hi @kashish2210 this looks good! A few comments for you to look at :)
test/test_events.jl
Outdated
|
|
||
| data_energy = readevents(energy_only_file) | ||
| @test isempty(data_energy.times) # Should be empty as no TIME column exists | ||
| @test length(data_energy.energies) == 3 # ENERGY data should be read even if TIME is missing |
There was a problem hiding this comment.
Without times, the energies are a little meaningless. This test should preferably raise an error if no TIME column is found in the FITS file.
| @test length(event_list.energies) == 10 | ||
| @test event_list.times == collect(1:10) | ||
| @test event_list.energies == collect(11:20) | ||
| @test length(event_list.metadata.headers) == 2 # Should only include headers up to first event HDU |
There was a problem hiding this comment.
Okay, you can likely ignore my earlier comment then.
There was a problem hiding this comment.
No, sorry I was referring to # Should only include headers up to first event HDU, so my comment about reading in metadata from additional HDUs can likely be ignored.
Co-authored-by: Fergus Baker <[email protected]>
Co-authored-by: Fergus Baker <[email protected]>
…y.jl into fix_issue#34
|
i guess thee problem has been soved in #47 so no need to open this |
4 participants
#34