pykoala / pykoala

Data reduction pipeline for KOALA on the AAT.
https://pykoala.readthedocs.io/en/latest/index.html
BSD 3-Clause "New" or "Revised" License
7 stars 17 forks source link

Unifying RSS and Cube classes #439

Closed paranoya closed 1 month ago

paranoya commented 3 months ago

In principle, many (if not most/all) corrections could be applied equally well to RSS and Cube objects. Usually, their intensity and variance attributes are simply treated as a collection of spectra. I will not insist on specutils.Spectrum1D (oops, I just did ;^D), but I think both objects have many attributes and methods in common, and that should be properly capture in the architecture. What about the following options? Option 1

class SpectraContainer(DataContainer):
    wavelength
    intensity
    variance
    get_rss_intensity()
    get_rss_variance()

class RSS(SpectraContainer):
    def get_rss_intensity(self):
        return self.intensity
    def get_rss_variance(self):
        return self.variance

class Cube(SpectraContainer):
    def get_rss_intensity(self):
        return np.reshape(self.intensity, (self.intensity.shape[0], self.intensity.shape[1]*self.intensity.shape[2]).T
    def get_rss_variance(self):
        return np.reshape(self.variance, (self.variance.shape[0], self.variance.shape[1]*self.variance.shape[2]).T

Option 2 Instead of a common parent (SpectraContainer), Cube could be a child from RSS and override get_rss_*

To be honest, I am more inclined towards Option 1, preparing for the time where other data types, such as astropy.CCDData (oops again ;^D) hang from DataContainer.

PabloCorcho commented 3 months ago

This made me laugh XD (shall we create an issue label for this as well?).

I like the getters idea and I am more inclined to the first option. This Friday I am attending an astropy meeting and I will try to bring up some discussion about this as well as learning about how to contribute.

In two weeks we could attempt to do the refactoring.