Time Series Builder for COSI GRBs:

[saurabhmittal23]

- Takes in a COSI binned dataset containing signal plus background before and after the GRB
- A response file (.hdf5 format or an OGIP compatible .rsp file)
- If response is .hdf5, requires an ori file and source location to correctly rotate the response and saves a .rsp file and 
   .arf file for future use
- Creates a threeML compatible time series object which can be used in threeML to perform spectral fits

[codecov]

Codecov Report

Attention: Patch coverage is 0% with 82 lines in your changes missing coverage. Please review.

Project coverage is 69.77%. Comparing base (d74b7cf) to head (d391137).
Report is 7 commits behind head on develop.

Files with missing lines	Patch %	Lines
cosipy/data_builder/Time_series_builder.py	0.00%	81 Missing ⚠️
cosipy/data_builder/__init__.py	0.00%	1 Missing ⚠️

Files with missing lines	Coverage Δ
cosipy/data_builder/__init__.py	0.00% <0.00%> (ø)
cosipy/data_builder/Time_series_builder.py	0.00% <0.00%> (ø)

... and 3 files with indirect coverage changes

[israelmcmc]

Hi @saurabhmittal23. Thanks for working on this! I'm sorry it took me a while, but I finally read your code in detail.

Although ultimately we want to use the full CDS for the GRB analysis, this code is useful because it will allow the user to do a simple ON/OFF analysis, which will be more familiar to many. It's also quicker, easier to visualize, and can be compared directly to other instruments.

I think that what we ultimately want to do is to add a function to 3ML itself, as suggested by the 3ML documentation:

Note: If you would like to build a time series from your own custom data, consider creating a TimeSeriesBuilder.from_your_data() class method.

I don't think we want to add the function to 3ML yet, because our data format is still in flux. You can however re-write your code so that it is a simple copy-past once we get to that point. You already have the code, it just needs to be refactored. Instead of creating a child class of TimeSeriesBuilder, we only need a single function, which you already have:

def from_cosi_grb_data(cls, *args, **kwargs):
    ...

This will be copy-pasted into 3ML's TimeSeriesBuilder (like all other missions), but for now it can be called with:

from threeML.utils.data_builders import TimeSeriesBuilder

cosi_data = from_cosi_grb_data(TimeSeriesBuilder, *args, **kwargs)

# Once from_cosi_grb_data is part of the 3ML code, the above is exactly the same as:
# cosi_data = TimeSeriesBuilder.from_cosi_grb_data(*args, **kwargs)

This is just using how Python knows about their own cls (for class methods) when called from the class itself, but otherwise, their class methods work like any other function.

In order to make this work, you need to move the code that you currently have in TimeSeriesBuilderCOSI.__init__() to from_cosi_grb_data() (right before your cls()) and instead of calling super(TimeSeriesBuilderCOSI, self).__init__() you'll use cls() (but this time cls = TimeSeriesBuilder). Everything else should be mostly the same.

Here are other comments besides this re-structuring:

• I'd use the more generic name from_cosi_data, that is, dropping the GRB part, since this could be use for other transients.
• I rather avoid defining another class for binned data, that is, COSIGRBData. Please use what's available for a generic binned data in DataIO. If there's something you need to achieve your goals that is not in DataIO, then feel free to modify that module. But again, it's something that can be used in general, not just GRBs.
• Please add a simple example of how to use this. I suggest adding a jupyter notebook here. Even better if you analyze one of the DC3 GRBs!
• Please add unit tests for your new code. You can find some help here.
• Eventually (this can moved to a future request issue), there should be the option to perform an ARM cut on the data. We really need this to reach something closer to the expected sensitivity of the instrument, even if it's a simple ON/OFF analysis.

Please don't hesitate to reach out if you need help with any of this.