Fix for TargetIOR1Calibrator

[watsonjj]

Fix for returning TargetIOR1Calibrator from CameraR1Calibrator.from_eventsource.

This is a temporary small fix. Need to consider a long term approach.

[codecov]

Codecov Report

Merging #1035 into master will decrease coverage by <.01%.
The diff coverage is 0%.

@@            Coverage Diff             @@
##           master    #1035      +/-   ##
==========================================
- Coverage   83.52%   83.52%   -0.01%     
==========================================
  Files         188      188              
  Lines       10655    10656       +1     
==========================================
  Hits         8900     8900              
- Misses       1755     1756       +1

Impacted Files	Coverage Δ
ctapipe/calib/camera/r1.py	93.33% <0%> (-2.13%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b40af75...649aebd. Read the comment docs.

[watsonjj]

I am encountering a very frustrating ImportError with the ctapipe_io_targetio plugin, regarding the TargetIOR1Calibrator:

(cta) 09:27 [Jason ~/Software/ctapipe_io_targetio]  (master)  $ python -c "from ctapipe.calib.camera.r1 import CameraR1Calibrator"
Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "/Users/Jason/Software/ctapipe/ctapipe/calib/__init__.py", line 5, in <module>
    from .camera import *
  File "/Users/Jason/Software/ctapipe/ctapipe/calib/camera/__init__.py", line 8, in <module>
    from .dl1 import *
  File "/Users/Jason/Software/ctapipe/ctapipe/calib/camera/dl1.py", line 14, in <module>
    from ...image import NeighbourPeakIntegrator, NullWaveformCleaner
  File "/Users/Jason/Software/ctapipe/ctapipe/image/__init__.py", line 1, in <module>
    from .hillas import *
  File "/Users/Jason/Software/ctapipe/ctapipe/image/hillas.py", line 11, in <module>
    from ..io.containers import HillasParametersContainer
  File "/Users/Jason/Software/ctapipe/ctapipe/io/__init__.py", line 12, in <module>
    detect_and_import_io_plugins()
  File "/Users/Jason/Software/ctapipe/ctapipe/core/plugins.py", line 16, in detect_and_import_io_plugins
    return detect_and_import_plugins(prefix='ctapipe_io_')
  File "/Users/Jason/Software/ctapipe/ctapipe/core/plugins.py", line 10, in detect_and_import_plugins
    in pkgutil.iter_modules()
  File "/Users/Jason/Software/ctapipe/ctapipe/core/plugins.py", line 11, in <dictcomp>
    if name.startswith(prefix)
  File "/Users/Jason/anaconda3/envs/cta/lib/python3.7/importlib/__init__.py", line 127, in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
  File "/Users/Jason/Software/ctapipe_io_targetio/ctapipe_io_targetio/__init__.py", line 15, in <module>
    from .calibration import TargetIOR1Calibrator
  File "/Users/Jason/Software/ctapipe_io_targetio/ctapipe_io_targetio/calibration.py", line 1, in <module>
    from ctapipe.calib import CameraR1Calibrator
ImportError: cannot import name 'CameraR1Calibrator' from 'ctapipe.calib' (/Users/Jason/Software/ctapipe/ctapipe/calib/__init__.py)

It happens because when importing the CameraR1Calibrator, the __init__.py files end up leading to import the io plugins, which in turns tries to import the TargetIOR1Calibrator, which requires CameraR1Calibrator to be defined.

I would really appreciate some assistance with this, as I am not finding a convenient solution.

[watsonjj]

I will open a new PR with a suggestion

[maxnoe]

I think this is a problem of strange hierarchy.

The problem here is, why is NeighborPeakIntegrator in image, if it's only needed by the calibrators?
This particular problem would be solved by moving NeighborPeakIntegrator to calib (Where it makes more sense anyway).

The other thing is, what we also already discussed somewhere: Wouldn't it make more sense, if the containers were defined, where they are used?
So e.g. the HillasParametersContainer in image/hillas.py and the StereoReconstructionContainer in reco/containers.py?
Then not all subpackages would import io, so the problem will be most likely solved completely

[kosack]

This was just because trace integration is not calibration, so it was a semantic reason to keep it in image (image is for all processing of image cubes)

[maxnoe]

But this creates the image cubes from the waveforms, right? So it's the step before any images exist.

[kosack]

the "waveforms" is what I mean by image cubes (it's just a 3D image, rather than 2D). Trace integration is closely coupled with image cleaning, especially if we implement the more-useful techniques that do both at the same time (which is in the plans)

[maxnoe]

Maybe split in two packages? image_extraction and image or image_analysis?

[maxnoe]

especially if we implement the more-useful techniques that do both at the same time (which is in the plans)

Any references on that? Dominik and me already asked for references on neighbor peak integration, but never got any.

For all I know, MAGIC and FACT have a clear separtation between extracting number of photons and arrival times from the waveforms and the pixel selection for reconstruction.

So it would be nice to be able to read about more advanced methods and not getting "It's a common method, it just works".

[kosack]

I have to think about it - that would work now, but I'm anticipating the implementation of e.g. 3D cleaning, and 2-step trace integration, so I just have to draw the relationship diagram to make sure there are no loops...

[kosack]

Yes, there's a reference for the 2-step waveform + image cleaning, but I have to look it up... it was written for an early NectarCam study I think, but it's the technique used in EventDisplay

[kosack]

basically it's an iterative approach to integrate -> clean -> re-integrate, ...

[kosack]

In any case, we need to write down the calibration and integration steps in a clear way somewhere as well, to be clear in what order we should compute and apply calibrations, gain-channel selections, trace integration + denoising

[kosack]

I'll create an issue, so we don't discuss it on a merged PR :)

[GernotMaier]

Yes, there's a reference for the 2-step waveform + image cleaning, but I have to look it up... it was written for an early NectarCam study I think, but it's the technique used in EventDisplay

Holder et al (2006), Astroparticle Physics 25, 391–401