Missing documentation for `scan_map_in_observations`

[ziotom78]

The docstring for the function scan_map_in_observations is missing some important details:

• It does not specify how the parameter maps should be formed. The source code reveals that if it's a dictionary, it should associate detector names with NumPy arrays of shape (3, NPIX) containing the I/Q/U maps, but this should be spelled explicitly.
• The function allows maps to be a plain NumPy array instead of a dictionary: how are multiple detectors handled in this case? Is the array supposed to be (3, NPIX) and the maps be used for all detectors, or should it be (NDET, 3, NPIX), with NDET the number of detectors?

• The code looks for a field Coordinates in the maps field. However, this clashes with the presence of the parameter input_map_in_galactic. What happens if the caller writes:

  scan_map_in_observations(
      obs,
      {
          "detA": mapA,
          "detB": mapB,
          "Coordinates": CoordinateSystem.Galactic,
      },
      input_map_in_galactic=False,
  )

  The code checks for the consistency of input_map_in_galactic and maps["Coordinates"], but if they aren't it prints a warning and continues the execution. It's not clear which of the two takes precedence. Perhaps it would be better to remove maps["Coordinates"] altogether?

[paganol]

Hi @ziotom78,

The docstring for the function scan_map_in_observations is missing some important details:

• It does not specify how the parameter maps should be formed. The source code reveals that if it's a dictionary, it should associate detector names with NumPy arrays of shape (3, NPIX) containing the I/Q/U maps, but this should be spelled explicitly.

I agree, I can add the same info we have in the documentation

• The function allows maps to be a plain NumPy array instead of a dictionary: how are multiple detectors handled in this case? Is the array supposed to be (3, NPIX) and the maps be used for all detectors, or should it be (NDET, 3, NPIX), with NDET the number of detectors?

Sure, we can add this extra possibility. The idea here was that when you pass a numpy array the same map is used for all the detectors. But we can add the possibility of having a (NDET, 3, NPIX) array as well. It seems a reasonable option.

• The code looks for a field Coordinates in the maps field. However, this clashes with the presence of the parameter input_map_in_galactic. What happens if the caller writes:

  scan_map_in_observations(
    obs,
    {
        "detA": mapA,
        "detB": mapB,
        "Coordinates": CoordinateSystem.Galactic,
    },
    input_map_in_galactic=False,
  )

  The code checks for the consistency of input_map_in_galactic and maps["Coordinates"], but if they aren't it prints a warning and continues the execution. It's not clear which of the two takes precedence. Perhaps it would be better to remove maps["Coordinates"] altogether?

Here the variable input_map_in_galactic is only relevant in two cases:

1. when you pass a numpy array to the field maps.
2. when you pass a dictionary, but the it hasn't the key Coordinates.

The last possibility, i.e. if the dictionary has the key Coordinates filled, input_map_in_galactic is ignored, and a warning appears. The logic is that, if we do not provide the coordinates in the input dictionary, the code should use input_map_in_galactic, otherwise it should use what is attached to the maps provided in the dictionary. Probably it is a bit convoluted, and we can improve it.

I can take care of the first two points, if you agree. For the third point, I gladly take advices.

[ziotom78]

Sure, we can add this extra possibility. The idea here was that when you pass a numpy array the same map is used for all the detectors. But we can add the possibility of having a (NDET, 3, NPIX) array as well. It seems a reasonable option.

No, sorry for the misunderstanding, I was not advocating to add this option. I was just commenting that the user might be confused whether the case should be (3, NPIX) or (NDET, 3, NPIX). I would advocate against a matrix of shape (NDET, 3, NPIX), as it would be hard to track bugs where one map is assigned to the wrong detectors. I believe we should just add a bit of information to the docstring.

The logic is that, if we do not provide the coordinates in the input dictionary, the code should use input_map_in_galactic, otherwise it should use what is attached to the maps provided in the dictionary.

Ok, I see. I guess it's just a matter of adding this information to the docstring.

I can take care of all the three points, as I have already partly implemented them in my working directory. Thanks!