Problem with "parent" argument in eventsource

[FrancaCassol]

Hi,
when I call the ctapipe_io_lst source from a tool:
flatfield_tool.run(argv=['--config','/astro/users/cassol/soft/python/cta-lstchain/lstchain/tools/flatfield_param.json'])

I get an error concerning the "parent" and the "config" arguments.

/scratch4/franca/soft/ctapipe/ctapipe/io/eventsource.py
 in __init__(self, config, parent, **kwargs)
    142         kwargs
    143         """
--> 144         super().__init__(config=config, parent=parent, **kwargs)
    145 
    146         self.metadata = dict(is_simulation=False)

/scratch4/franca/soft/ctapipe/ctapipe/core/component.py in __init__(self, config, parent, **kwargs)
     95         """
     96         if parent is not None and config is not None:
---> 97             raise ValueError('Only one of `config` or `parent` allowed')
     98         super().__init__(parent=parent, config=config, **kwargs)
     99 
ValueError: Only one of `config` or `parent` allowed

The Component argument description seems outdated (there is "tool" not "parent").
Can perhaps somebody help?
Thanks

[maxnoe]

Hi Franca, I already opened a PR in the LST eventsource to fix this: cta-observatory/ctapipe_io_lst#6

You maybe need to click the "watch" button in this repository, so you receive notifications.

[kosack]

Perhaps we should rights to merge things in that repo to Franca (or whoever is maintaining it)?

[kosack]

In any case, I just merged it

[maxnoe]

Perhaps we should rights to merge things in that repo to Franca (or whoever is maintaining it)?

I gave her Admin permissions a few days ago

[FrancaCassol]

Thanks Karl,
unfortunately with my tool I continue to get the error :

ValueError: Only one of config or parent allowed

flatfield_tool.run(argv=['--config','/astro/users/cassol/soft/python/cta-lstchain/lstchain/tools/flatfield_param.json'])

---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
<ipython-input-3-0f1913781b4d> in <module>()
      1 #
----> 2 flatfield_tool.run(argv=['--config','/astro/users/cassol/soft/python/cta-lstchain/lstchain/tools/flatfield_param.json'])

/scratch4/franca/soft/ctapipe/ctapipe/core/tool.py in run(self, argv)
    163             Provenance().start_activity(self.name)
    164             Provenance().add_config(self.config)
--> 165             self.setup()
    166             self.is_setup = True
    167             self.start()

~/soft/python/cta-lstchain/lstchain/tools/calc_flatfield.py in setup(self)
     72         self.flatfield = FlatFieldCalculator.from_name(
     73             self.calculator_product,
---> 74             **kwargs
     75         )
     76         self.cleaner = WaveformCleaner.from_name(

/scratch4/franca/soft/ctapipe/ctapipe/core/component.py in from_name(cls, name, config, parent)
    141         requested_subclass = subclasses[name]
    142 
--> 143         return requested_subclass(config=config, parent=parent)

~/soft/python/cta-lstchain/lstchain/calib/camera/flatfield.py in __init__(self, **kwargs)
    109         Parameters: see base class FlatFieldCalculator
    110         """
--> 111         super().__init__(**kwargs)
    112 
    113         self.log.info("Used events statistics : %d", self.sample_size)

~/soft/python/cta-lstchain/lstchain/calib/camera/flatfield.py in __init__(self, **kwargs)
     73 
     74         """
---> 75         super().__init__(**kwargs)
     76 
     77         # initialize the output

/scratch4/franca/soft/ctapipe/ctapipe/core/component.py in __init__(self, config, parent, **kwargs)
     95         """
     96         if parent is not None and config is not None:
---> 97             raise ValueError('Only one of `config` or `parent` allowed')
     98         super().__init__(parent=parent, config=config, **kwargs)
     99 

ValueError: Only one of `config` or `parent` allowed

That is the same for the ChargeResolutionGenerator tool of ctapipe that I took as example
Actually this is not surprising because I am sending both config and parent when I define:
kwargs = dict(config=self.config, parent=self)

should I not better define
kwargs = dict(config=self.config, parent=None)?

Then it works

[maxnoe]

You should do it the other way around. Only pass parent=self

[maxnoe]

We updated all the tools in ctapipe, so maybe just have a look at the tool you adapted

[FrancaCassol]

That is what I did, did you test the ChargeResolutionGenerator tool?

There is also a problem with one of the arguments:

from ctapipe.tools.extract_charge_resolution import ChargeResolutionGenerator tool= ChargeResolutionGenerator() tool.print_help()

`Calculate the Charge Resolution from a sim_telarray simulation and store within
a HDF5 file.

Options

Arguments that take values are actually convenience aliases to full
Configurables, whose aliases are listed on the help line. For more information
on full configurables, see '--help-all'.

---

KeyError Traceback (most recent call last)
in ()
2 from ctapipe.tools.extract_charge_resolution import ChargeResolutionGenerator
3 tool= ChargeResolutionGenerator()
----> 4 tool.print_help()

/scratch4/CTA_soft/anaconda3/envs/cta-dev/lib/python3.6/site-packages/traitlets/config/application.py in print_help(self, classes)
384 self.print_description()
385 self.print_subcommands()
--> 386 self.print_options()
387
388 if classes:

/scratch4/CTA_soft/anaconda3/envs/cta-dev/lib/python3.6/site-packages/traitlets/config/application.py in print_options(self)
355 print(os.linesep.join(lines))
356 self.print_flag_help()
--> 357 self.print_alias_help()
358 print()
359

/scratch4/CTA_soft/anaconda3/envs/cta-dev/lib/python3.6/site-packages/traitlets/config/application.py in print_alias_help(self)
319 for alias, longname in self.aliases.items():
320 classname, traitname = longname.split('.',1)
--> 321 cls = classdict[classname]
322
323 trait = cls.class_traits(config=True)[traitname]

KeyError: 'PeakFindngIntegrator'

`

[FrancaCassol]

Anyway, thanks, I will change as you suggest.
Just, it would be better to update the Component documentation in order to explain correctly (and clearly) the arguments expected. Now it is still related to the old version:

    """
    Parameters
    ----------
    config : traitlets.loader.Config
        Configuration specified by config file or cmdline arguments.
        Used to set traitlet values.
        Set to None if no configuration to pass.
    tool : Tool or Component
        Tool or component that is the Parent of this one
    kwargs
        Traitlets to be overridden.
        TraitError is raised if kwargs contains a key that does not
        correspond to a traitlet.
    """

[maxnoe]

The help string you posted is correct. Both options are necessary but mutual exclusive.

If you have a Tool or Component, that has other Compents as members, these must get parent=self,
if you have a Component on its own, pass config

[FrancaCassol]

Actually the argument "tool" does not exist anymore, also a bit strange the concept of "both necessary but mutual exclusive" (for example I used only on them) ...
But indeed, I think it would be very helpful to add your last phrase to the documentation:

If you have a Tool or Component, that has other Compents as members, these must get parent=self, if you have a Component on its own, pass config

[maxnoe]

Both parameters are needed for the class, they do different things.
We had a larger discussion here, it boils down to traitlets implementation details.
E.g. here #958

TL:DR: passing both config and parent does strange things, this is why we now forbid it.

If there is a parent Component, child components must get parent and not config, if there is no parent, one can pass config.

[maxnoe]

Ah, I just saw that the docstring says tool, that's wrong.

[maxnoe]

See #1016

[thomasgas]

This issue can be closed after #1016?

[dneise]

I think this can be closed now @FrancaCassol ?