Consistency between MapDataset.stack and Datasets.stack_reduce about gammapy HOT 3 OPEN

If I remember correctly, the behaviour in MapDataset.stack is intentional. It was the cleanest way to be able to stack maps during the data reduction process while handling the safe mask correctly.
This is included in the Note here https://docs.gammapy.org/1.2/user-guide/datasets/index.html#stacking-multiple-datasets - "To properly handle masks, it is necessary to stack onto an empty dataset."
Datasets.stack_reduce was introduced to avoid users stacking onto non-empty maps. The idea was general users should only use Datasets.stack_reduce and not MapDataset.stack. I guess we did not expose these consistently.

Not sure what is happening in MapDatasetOnOff.

from gammapy.

Using MapDataset.stack the mask is not applied to self, the assumption being that the first dataset used is empty.

If I remember correctly, the first implementation of MapDataset.stack() had the in-place application of the mask. This lead to a lot of complex code, because one needed to apply the mask to all quantities in the dataset. The same code was then repeated for the "other" dataset. It was much simpler to introduce the .to_masked() method, with all the mask application in a single place.

Applying the mask for in-place stacking would mean to always apply it, because one cannot know whether the stacking happens the first time. This means, except for the first time, one would un-necessarily apply the (stacked) mask over and over again.

But independently, stack_reduce() is the higher level method anyway. So I think there is no need for consistency. However the behavior of the on-off dataset should be fixed.

from gammapy.

OK thanks. Not re-masking all the time seems a reasonable approach indeed. While solving #5245 , I try to adapt the docstring and the behaviour of MapDatasetOnOff.stack()

from gammapy.