ctools

I am trying to change the inheritance of the cscripts from gammalib.GApplication to ctools.ctool. However, calling the ctool.__init__() function in the cscript constructor raises an error:

AttributeError: No constructor defined - class is abstract
Exception TypeError: "in method 'GApplication_logTerse', argument 1 of type 'GApplication const *'" in <bound method csresmap.__del__ of <__main__.csresmap;  >> ignored

Can you push your current working branch into git so that I can have a look?

This is indeed a tricky problem that I encountered already when I wrote the GTestSuite class.

My understanding of the problem is that Python does not know the concept of an abstract class, hence you cannot derive directly from ctool.i. The work around to this is to create a non-abstract class that derives from ctool.i and then to make a Python interface for this non-abstract class.

I did this by adding the cscript class to the ctool.i file. You will see that the cscript class exists twice in ctool.i. Once at the top within the { }; brackets which is basically C++ code that is copy-pasted into the SWIG interface. Hence in this top section I create a C++ cscript class that implements dummy versions of the pure virtual methods that basically do nothing. In that way, cscript is a non-abstract class that shares all methods with ctool. The at the bottom there is the Python interface to this class that SWIG needs to create the corresponding Python classes.

Now check csresmap. You will see

class csresmap(ctools.cscript):

        # Initialise application
        if len(argv) == 0:
            ctools.cscript.__init__(self, self.name, self.version)
        elif len(argv) ==1:
            ctools.cscript.__init__(self, self.name, self.version, *argv)
        else:
            raise TypeError("Invalid number of arguments given.")

There was a conflict when I merged in the code which I corrected. Hence when you continue with adapting all cscripts please get a fresh checkout from devel.

I also added the

pars.append(gammalib.GApplicationPar("logfile","f","h","csresmap.log","","","Log filename"))

• cresmap: already done
• cscaldb: doesn’t use any functionality to set up observations or maps. Therefore, it could stay as it is, inheriting from gammalib.GApplication?
• cspull, cstsdist: They used different approaches to setup the observation container (all using obsutils functionality). I made them derived classes from cscript and slightly changed the parameter structure. Now the observation setup is more homogeneous like in ctool. I however kept the original philosophy by setting some parameters to hidden (e.g. “ra”, “dec” in cstsdist, or “inobs” in cspull). Furthermore I hid the parameter “tmin” (with 0.0 as default). At the same time, “duration” had to be changed to “tmax” to be conform with ctool nomenclature.
• cssens: I left this tool untouched because observations are setup in a for-loop with different energy ranges. Accordingly, using ctool-functionality here might complicate this tool in my view.

Generally, we have to decide whether all cscripts should derive from ctools.cscript or only if the functionality of the base class is used after all.
I adapted the test cases according to the new parameters and pushed the changed to the working branch

I would opt for deriving all scripts from cscript so that we could eventually adapt this class so that all cscripts fulfill some common interface or behavior.

Agreed, I adapted the code accordingly.

Some of the actual scripts are quite old, and it may be worth to see if it makes sense to rewrite them with the current new logic. This is however not an urgent issue.

I realised that when going through the code . E.g. for cstsdist, the ts computation functionality was not used so far. However, into the output file, some more values are written which would have been lost if we used the new algorithm (e.g. npred_nullhypothesis).

Apart from that everything should work fine now with the ctool.cscript inheritance.

Merged into devel.