jgliss / pyplis

Python toolbox for the analysis of UV SO2 camera data
GNU General Public License v3.0
7 stars 5 forks source link

Conceptual behaviour of `BaseImgList._iter_num` #17

Open solvejgdinger opened 5 years ago

solvejgdinger commented 5 years ago

I have a question regarding the implmentation of BaseImgList._iter_num in the imagelists.py module. From the documentation it is not clear whether the image at stop_idx is included. Consequently, it is neither clear for methods (averaging, make_stack, ...) making use of it. It might be that it is not consistently used throughout pyplis.

Source:

def _iter_num(self, start_idx, stop_idx):
        """Return the number of iterations for a loop.

        The number of iterations is based on the current attribute
        ``skip_files``.

        Parameters
        ----------
        start_idx : int
            start index of loop
        stop_idx : int
            stop index of loop

        Returns
        -------
        int
            number of required iterations

        """
        # the int(x) function rounds down, so no floor(x) needed
        return int((stop_idx - start_idx) / (self.skip_files + 1.0))

Many methods are implemented as

self.goto_img(start_idx)
for k in range(self._iter_num(start_idx, stop_idx)):
    < do something >
    self.goto_next()

Hence it would reach the last image at the end of the last loop iteration and consequently not consider the last image. For example, if I have a ImgList with 6 images, my index would be [0,1,2,3,4,5]. Calling self._iter_num(0, self.nof) returns 6 and it would go correctly 6 times through the loop. This includes the las image only because self.nof is not the index of the last image. Instead, calling self._iter_num(0, 5) returns 5, so looping only over 5 images and ignoring the last index.

So my question is if this is a bug (can be solved easily) or if the documentation in other methods (e.g. self.make_stack) is wrong. Or worse, it is used sometimes in one way and sometimes in another. I have avoided the method for this reason in the past, but it's good to know anyway. ;-)

jgliss commented 2 years ago

@solvejgdinger I would need to check this, but could well be, that this is a bug, I would say the default behaviour should loop over all images...

jgliss commented 2 years ago

on the other hand, thinking about it a little more, for image list processing, it should only go until the second last image in case optical flow is supposed to be calculated, because for the very last image, optical flow cannot be calculated