GammaLib

Jürgen Knödlseder wrote:

In principle, I always use a 4 space Tab for writing the Python code, but there can be mixes with the old code.

I was not aware of the Python style guide, thanks for pointing me to the reference. I guess we should add this to the Coding Conventions. We can should add a chapter “2.2 Python rules” to the document.

Can you go ahead and transform the code and also add a chapter about Python coding rules?

Will do tomorrow.
Merging it into your several major branches might not be trivial, I’ll simply branch devel and let’s see if everything works well.

I created feature #618 that asks for inclusion of the PEP8 rules in Jenkins.

Thanks

Do you know if there are automatic tools to check C++ coding rules?

There must be, I added this to #559

Christoph Deil wrote:

Jürgen Knödlseder wrote:

In principle, I always use a 4 space Tab for writing the Python code, but there can be mixes with the old code.

I was not aware of the Python style guide, thanks for pointing me to the reference. I guess we should add this to the Coding Conventions. We can should add a chapter “2.2 Python rules” to the document.

Can you go ahead and transform the code and also add a chapter about Python coding rules?

Will do tomorrow.
Merging it into your several major branches might not be trivial, I’ll simply branch devel and let’s see if everything works well.

Right, the idea was that you should always base new code on devel, and also merge back into devel. Actually, hess and comptel are pretty synchronized, comptel will soon disappear as I’m about to finish a first COMPTEL interface (I’m looking forward to see the first Crab spectrum from 750 keV to 20 TeV obtained with GammaLib - maybe still tonight

I created feature #618 that asks for inclusion of the PEP8 rules in Jenkins.

Thanks

Do you know if there are automatic tools to check C++ coding rules?

There must be, I added this to #559

Great.

Which Python versions should gammalib support?
My suggestion would be Python 2.6, 2.7, 3.2 and 3.3.

I can put this info in the coding guidelines and the install instructions.
Jenkins only runs 2.7 and 3.2 at the moment, could it be changed to run all supported versions?
One architecture / compiler should be enough here.

Python 2.6 is pretty late, I still have systems running Python 2.4! I would simply say Python 2 and 3, although I’m not sure that we have to go before 2.4 ...

For the moment I’m only testing 2.73 and 3.23 (see https://cta-jenkins.irap.omp.eu/view/CTA%20Python/job/gammalib-build-python/), but this list can be extended (just have to compile the other Python versions on the test machines).

The reason why I use several architectures / compilers is related to swig. The Python interface is built with swig, which implies compilation of wrapper code. I want to make sure that this works nice with different compilers and architectures.

By the way: your new check seems to fail on Python 3 (see https://cta-jenkins.irap.omp.eu/view/CTA%20Python/job/gammalib-check-python/):

basic_string::_S_construct null not valid

I opened bug #624 and assigned it to you (pull from devel before working on the code as I did some editing of the test_GSky.py file).

Pull request: pep8 branch
https://cta-redmine.irap.omp.eu/projects/gammalib/repository/show?rev=pep8

Note that this branch has to be rebased on devel before it can be merged.
So far I haven’t managed to rebase (because of conflicts with the COMPTEL changes), but I’ll keep trying ...

I ran these commands:

autopep8 --ignore=E501 --in-place pyext/check_config.py pyext/setup.py dev/replace_license.py
autopep8 --ignore=E501 --in-place --recursive inst
autopep8 --ignore=E501 --in-place --recursive test

Then I fixed these three things by hand which autopep8 didn’t fix:

$ pep8 . --ignore=E501 --exclude=pyext
./inst/lat/test/binned_integration.py:108:21: E127 continuation line over-indented for visual indent
./test/test_GSky.py:93:16: E221 multiple spaces before operator
./test/test_GSky.py:107:16: E221 multiple spaces before operator

There was an IndentationError in test/test_file_function.py. Should this test be run?

Finally I added a short Section to the CDC document on Python versions and style.