ctools

Moved into devel - for now.

I’m still not sure about the name of the method (pars). The reason is that GApplication has a pars() method that returns the parameters, and in Python this method is renamed to protected _pars(). But then a pars() method is added in Python that gives access to the dictionary. I’m not sure that (almost) the same name should be used for two methods that take or return different types.

I better find some better names

What about pardict() in Python? Makes clear that we expect and get a dictionary.

What about pardict() in Python? Makes clear that we expect and get a dictionary.

Yes that sounds clearer.

I like that the following code is now possible:

import ctools
mytool = "ctobssim" 
tool = ctools.__dict__[mytool]()
for par in tool:
    print(par) # or do sth else
tool.run()

We should add some example code for this on the documentation as well.

Mayer Michael wrote:

What about pardict() in Python? Makes clear that we expect and get a dictionary.

Yes that sounds clearer.

I like that the following code is now possible:
[...]
This way people could easily add support for simple ascii config files, too.

We should add some example code for this on the documentation as well.

I tried to run you few lines of code but could not manage to make it work. How should this example work?

I tried to run you few lines of code but could not manage to make it work. How should this example work?

Apologies, I made a typo. The for loop should of course look like this:

for par in tool._pars():
    print(par)

for par in tool:

Mayer Michael wrote:

I tried to run you few lines of code but could not manage to make it work. How should this example work?

Apologies, I made a typo. The for loop should of course look like this:
[...]
I realised that tool.pars() doesn’t work but we need the underscore - any idea why?

I made all public methods of the GApplication base class private for ctools (by prepending an underscore), mainly because the code quality checker (pylint) complained about having too many public methods. The reason behind not having too many public methods is maintenance of the public interface.

I’m still a bit hesitant about this public versus private thing in Python. Python has not really private methods since a user can access all methods, but the convention - recognised by pylint - is that prepending an underscore means that a method is private. There are methods that a user probably should not really use (such as the application logger), but I agree that access to the parameter interface is useful for a user.

You can see how this works in GApplication.i:

    // Ignore base class methods and make methods private in Python by
    // prepending an underscore
    %ignore                  clear;
    %ignore                  clone;
    %ignore                  classname;
    %rename(_name)           name;
    %rename(_version)        version;
    %rename(_logTerse)       logTerse;
    %rename(_logNormal)      logNormal;
    %rename(_logExplicit)    logExplicit;
    %rename(_logVerbose)     logVerbose;
    %rename(_logDebug)       logDebug;
    %rename(_clobber)        clobber;
    %rename(_has_par)        has_par;
    %rename(_par_filename)   par_filename;
    %rename(_log_filename)   log_filename;
    %rename(_log_header)     log_header;
    %rename(_log_trailer)    log_trailer;
    %rename(_need_help)      need_help;
    %rename(_log)            log;
    %rename(_pars)           pars;

It might even be useful to add the operator[int index] to GApplication to allow a direct loop over the parameters via [...] What do you think?

I agree that this would be useful. Note that to make it work as an iterator you have to add an exception in case that the index is out of range (that’s how Python iterators detect the end). An example is in GModels.i:

    GModel* __getitem__(const int& index) {
        if (index >= 0 && index < self->size()) {
            return (*self)[index];
        }
        else {
            throw GException::out_of_range("__getitem__(int)", "Model index",
                                           index, self->size());
        }
    }
    void __setitem__(const int& index, const GModel& val) {
        if (index >= 0 && index < self->size()) {
            self->set(index, val);
            return;
        }
        else {
            throw GException::out_of_range("__setitem__(int)", "Model index",
                                           index, self->size());
        }
    }

Ok, I can make the change to the GApplication interface.
Concerning the unit tests: I believe a simple loop in Python from start to end should be sufficient. All these tests will thus be in $GammaLib/test/test_*.py files, right?

Mayer Michael wrote:

Ok, I can make the change to the GApplication interface.
Concerning the unit tests: I believe a simple loop in Python from start to end should be sufficient. All these tests will thus be in $GammaLib/test/test_*.py files, right?

Thanks!

For the unit test you may check test_GSky.py which has a corresponding test for the GSkyMap class:

        # Access operator (tests also proper iteration)
        sum = 0.0
        for pix in map:
            sum += pix
        self.test_value(sum, 16.0)        

Modifications are on branch 1838-access-GApplicationPars (in GammaLib).
Note that I added an iterator test to test_GModels.py as well.