New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add EnergyOffsetArray #383

Merged
merged 48 commits into from Feb 1, 2016

Conversation

Projects
None yet
4 participants
@JouvinLea
Contributor

JouvinLea commented Nov 20, 2015

Add a class that represents an array with log(energy) and offset axes.

@cdeil

This comment has been minimized.

Show comment
Hide comment
@cdeil

cdeil Nov 25, 2015

Member

@JouvinLea – I'll think a bit more about how this class should work and leave notes -- we probably want a generic "array with axes" object similar to ROOT histograms as base class, but that is not a good first project / pull request, because it's not clear yet how to best implement it.

If you have time to get started with this, my suggestion would be that you implement code that produces the Figures shown here: #297

What's needed is:

  • Add a EnergyOffsetArray.fill method that takes an EventList, computes offset for each event (use EventList.pointing_radec and EventList.radec call SkyCoord.separation) and then uses numpy.histogramdd to fill the (energy, offset) values in the array.
  • Add a EnergyOffsetArray.plot_image method that does something very similar to AreaTable2D.plot_image. (we should then add a EffectiveAreaTable2D.energy_offset_array property to avoid the duplication of code, but this is something we can do later, not in this pull resuts).
  • Add a test for your new class

I'd prefer if we leave it at that for this pull request, and then you continue extending this class with FITS I/O and background modeling functionality in future separate pull requests.
But of course, if you prefer, feel free to implement as much functionality as you like here.

Let me know if you have any questions how to get started with this!

Member

cdeil commented Nov 25, 2015

@JouvinLea – I'll think a bit more about how this class should work and leave notes -- we probably want a generic "array with axes" object similar to ROOT histograms as base class, but that is not a good first project / pull request, because it's not clear yet how to best implement it.

If you have time to get started with this, my suggestion would be that you implement code that produces the Figures shown here: #297

What's needed is:

  • Add a EnergyOffsetArray.fill method that takes an EventList, computes offset for each event (use EventList.pointing_radec and EventList.radec call SkyCoord.separation) and then uses numpy.histogramdd to fill the (energy, offset) values in the array.
  • Add a EnergyOffsetArray.plot_image method that does something very similar to AreaTable2D.plot_image. (we should then add a EffectiveAreaTable2D.energy_offset_array property to avoid the duplication of code, but this is something we can do later, not in this pull resuts).
  • Add a test for your new class

I'd prefer if we leave it at that for this pull request, and then you continue extending this class with FITS I/O and background modeling functionality in future separate pull requests.
But of course, if you prefer, feel free to implement as much functionality as you like here.

Let me know if you have any questions how to get started with this!

@cdeil cdeil assigned cdeil and unassigned adonath Nov 25, 2015

@cdeil cdeil changed the title from implement energy_array_class to Add EnergyOffsetArray Nov 25, 2015

@cdeil cdeil added the feature label Nov 25, 2015

@cdeil cdeil added this to the 0.4 milestone Nov 25, 2015

@cdeil

This comment has been minimized.

Show comment
Hide comment
@cdeil

cdeil Jan 18, 2016

Member

@JouvinLea - I just now noticed this issue:
https://travis-ci.org/gammapy/gammapy/jobs/96068721

Can you please try this and see if it fixes it?

git checkout master
git pull # get the latest master version
git checkout BackgroundModelling
git rebase master # rebase this branch on master ... there should be no conflicts, it'll just run through
git push -f JouvinLea  BackgroundModelling

The last command force pushes the rebased branch BackgroundModeling to the remove JouvinLea on Github (your fork, this PR).
I'm not sure if JouvinLea is the correct name for your fork ... you can find out by typing

git remote -v
Member

cdeil commented Jan 18, 2016

@JouvinLea - I just now noticed this issue:
https://travis-ci.org/gammapy/gammapy/jobs/96068721

Can you please try this and see if it fixes it?

git checkout master
git pull # get the latest master version
git checkout BackgroundModelling
git rebase master # rebase this branch on master ... there should be no conflicts, it'll just run through
git push -f JouvinLea  BackgroundModelling

The last command force pushes the rebased branch BackgroundModeling to the remove JouvinLea on Github (your fork, this PR).
I'm not sure if JouvinLea is the correct name for your fork ... you can find out by typing

git remote -v
@JouvinLea

This comment has been minimized.

Show comment
Hide comment
@JouvinLea

JouvinLea Jan 18, 2016

Contributor

I did it! Is it working?
Le 18 janv. 2016 à 11:31, Christoph Deil notifications@github.com a écrit :

git push -f JouvinLea BackgroundModelling

Contributor

JouvinLea commented Jan 18, 2016

I did it! Is it working?
Le 18 janv. 2016 à 11:31, Christoph Deil notifications@github.com a écrit :

git push -f JouvinLea BackgroundModelling

@cdeil

This comment has been minimized.

Show comment
Hide comment
@cdeil

cdeil Jan 18, 2016

Member

Yes, it worked.

You can always check yourself for the pull requests (PRs) you're working on.

In this case go to #383 and look at the continuous integration (CI) checks at the bottom.

For the last commit you pushed (c23f886) the builds are here:
https://travis-ci.org/gammapy/gammapy/builds/103088181

Checking the first one that failed (i.e. that's red) you'll see this error:
https://travis-ci.org/gammapy/gammapy/jobs/103088185#L641

So you have to adapt gammapy/background/energy_offset_array.py line five and change

from ..obs import DataStore

to

from ..data import DataStore

because I moved that class to gammapy.data.DataStore.
Or better, as we discussed ... change the fill method to just take a list of EventList objects as input, not a DataStore.

In this case, you should see the same error locally, so that's quicker to debug than via travis-ci.
If you don't see the error, try

make clean

to clean out old files that might still be around on your system.

Usually the travis-ci tests run within ~ 10 minutes, so after pushing changes, you'll have to wait a bit and do something else before the CI test results are available. Usually that's not an issue because you can continue with local code, tests, docs and most of the issues also occur locally.

So first goal here: get the tests to pass again, even if the test function is empty.
Then: continue with the implementation / test for this class, using one or two Crab runs as an example.

Member

cdeil commented Jan 18, 2016

Yes, it worked.

You can always check yourself for the pull requests (PRs) you're working on.

In this case go to #383 and look at the continuous integration (CI) checks at the bottom.

For the last commit you pushed (c23f886) the builds are here:
https://travis-ci.org/gammapy/gammapy/builds/103088181

Checking the first one that failed (i.e. that's red) you'll see this error:
https://travis-ci.org/gammapy/gammapy/jobs/103088185#L641

So you have to adapt gammapy/background/energy_offset_array.py line five and change

from ..obs import DataStore

to

from ..data import DataStore

because I moved that class to gammapy.data.DataStore.
Or better, as we discussed ... change the fill method to just take a list of EventList objects as input, not a DataStore.

In this case, you should see the same error locally, so that's quicker to debug than via travis-ci.
If you don't see the error, try

make clean

to clean out old files that might still be around on your system.

Usually the travis-ci tests run within ~ 10 minutes, so after pushing changes, you'll have to wait a bit and do something else before the CI test results are available. Usually that's not an issue because you can continue with local code, tests, docs and most of the issues also occur locally.

So first goal here: get the tests to pass again, even if the test function is empty.
Then: continue with the implementation / test for this class, using one or two Crab runs as an example.

@JouvinLea

This comment has been minimized.

Show comment
Hide comment
@JouvinLea

JouvinLea Jan 18, 2016

Contributor

with the rebase I have this error now
cannot import name ring_area_factor
when I call:
from ..background import ring_area_factor, Cube
I don’t understand because the function is well defined in the class background in ring.py?

Contributor

JouvinLea commented Jan 18, 2016

with the rebase I have this error now
cannot import name ring_area_factor
when I call:
from ..background import ring_area_factor, Cube
I don’t understand because the function is well defined in the class background in ring.py?

@cdeil

This comment has been minimized.

Show comment
Hide comment
@cdeil

cdeil Jan 18, 2016

Member

The gammapy.background.ring_area_factor function is there.

Where do you get this error?
Can you push the latest version of the code to this branch on Github, and then point out the line where it happens and the stack trace you get?

Member

cdeil commented Jan 18, 2016

The gammapy.background.ring_area_factor function is there.

Where do you get this error?
Can you push the latest version of the code to this branch on Github, and then point out the line where it happens and the stack trace you get?

@JouvinLea

This comment has been minimized.

Show comment
Hide comment
@JouvinLea

JouvinLea Jan 18, 2016

Contributor

Sorry I was in a meeting it took to much time!
I did the pull request! I don’t know how to point out the problem?
Apparently it happened in the class spectrum_analysis.py due to this line: from ..background import ring_area_factor, Cube
but I don’t understand since the function ring_area_factor is well defined...

Contributor

JouvinLea commented Jan 18, 2016

Sorry I was in a meeting it took to much time!
I did the pull request! I don’t know how to point out the problem?
Apparently it happened in the class spectrum_analysis.py due to this line: from ..background import ring_area_factor, Cube
but I don’t understand since the function ring_area_factor is well defined...

@JouvinLea

This comment has been minimized.

Show comment
Hide comment
@JouvinLea

JouvinLea Jan 18, 2016

Contributor

and when I am running the test I get these errors...

skipping 'gammapy/detect/_test_statistics_cython.c' Cython extension (up-to-date)
INTERNALERROR> Traceback (most recent call last):
INTERNALERROR>   File "_pytest.main", line 80, in wrap_session
INTERNALERROR>     config.do_configure()
INTERNALERROR>   File "_pytest.config", line 621, in do_configure
INTERNALERROR>     self.hook.pytest_configure(config=self)
INTERNALERROR>   File "_pytest.core", line 521, in __call__
INTERNALERROR>     return self._docall(self.methods, kwargs)
INTERNALERROR>   File "_pytest.core", line 528, in _docall
INTERNALERROR>     firstresult=self.firstresult).execute()
INTERNALERROR>   File "_pytest.core", line 394, in execute
INTERNALERROR>     res = method(*args)
INTERNALERROR>   File "gammapy/conftest.py", line 40, in pytest_configure
INTERNALERROR>     from .utils.testing import has_data
INTERNALERROR>   File "gammapy/utils/testing.py", line 7, in <module>
INTERNALERROR>     from ..data import DataManager
INTERNALERROR>   File "gammapy/data/__init__.py", line 16, in <module>
INTERNALERROR>     from .spectral_cube import *
INTERNALERROR>   File "gammapy/data/spectral_cube.py", line 18, in <module>
INTERNALERROR>     from ..spectrum import LogEnergyAxis, powerlaw
INTERNALERROR>   File "gammapy/spectrum/__init__.py", line 19, in <module>
INTERNALERROR>     from .spectrum_analysis import *
INTERNALERROR>   File "gammapy/spectrum/spectrum_analysis.py", line 12, in <module>
INTERNALERROR>     from ..background import ring_area_factor, Cube
INTERNALERROR>   File "gammapy/background/__init__.py", line 10, in <module>
INTERNALERROR>     from .ring import *
INTERNALERROR>   File "gammapy/background/ring.py", line 131
INTERNALERROR>     return (r_out ** 2 - r_in ** 2) / theta ** 2
INTERNALERROR>                                                ^
INTERNALERROR> IndentationError: unindent does not match any outer indentation level
Contributor

JouvinLea commented Jan 18, 2016

and when I am running the test I get these errors...

skipping 'gammapy/detect/_test_statistics_cython.c' Cython extension (up-to-date)
INTERNALERROR> Traceback (most recent call last):
INTERNALERROR>   File "_pytest.main", line 80, in wrap_session
INTERNALERROR>     config.do_configure()
INTERNALERROR>   File "_pytest.config", line 621, in do_configure
INTERNALERROR>     self.hook.pytest_configure(config=self)
INTERNALERROR>   File "_pytest.core", line 521, in __call__
INTERNALERROR>     return self._docall(self.methods, kwargs)
INTERNALERROR>   File "_pytest.core", line 528, in _docall
INTERNALERROR>     firstresult=self.firstresult).execute()
INTERNALERROR>   File "_pytest.core", line 394, in execute
INTERNALERROR>     res = method(*args)
INTERNALERROR>   File "gammapy/conftest.py", line 40, in pytest_configure
INTERNALERROR>     from .utils.testing import has_data
INTERNALERROR>   File "gammapy/utils/testing.py", line 7, in <module>
INTERNALERROR>     from ..data import DataManager
INTERNALERROR>   File "gammapy/data/__init__.py", line 16, in <module>
INTERNALERROR>     from .spectral_cube import *
INTERNALERROR>   File "gammapy/data/spectral_cube.py", line 18, in <module>
INTERNALERROR>     from ..spectrum import LogEnergyAxis, powerlaw
INTERNALERROR>   File "gammapy/spectrum/__init__.py", line 19, in <module>
INTERNALERROR>     from .spectrum_analysis import *
INTERNALERROR>   File "gammapy/spectrum/spectrum_analysis.py", line 12, in <module>
INTERNALERROR>     from ..background import ring_area_factor, Cube
INTERNALERROR>   File "gammapy/background/__init__.py", line 10, in <module>
INTERNALERROR>     from .ring import *
INTERNALERROR>   File "gammapy/background/ring.py", line 131
INTERNALERROR>     return (r_out ** 2 - r_in ** 2) / theta ** 2
INTERNALERROR>                                                ^
INTERNALERROR> IndentationError: unindent does not match any outer indentation level
@cdeil

This comment has been minimized.

Show comment
Hide comment
@cdeil

cdeil Jan 18, 2016

Member

@JouvinLea - I think for some reason your Gammapy working director contains changes unrelated to this PR that are causing the errors you mention.

The last error you mentioned is

NTERNALERROR>   File "gammapy/background/ring.py", line 131
INTERNALERROR>     return (r_out ** 2 - r_in ** 2) / theta ** 2
INTERNALERROR>                                                ^
INTERNALERROR> IndentationError: unindent does not match any outer indentation level

In this branch and Gammapy master there is no IndentationError in this file / function:
https://github.com/gammapy/gammapy/blob/master/gammapy/background/ring.py#L131

Can you try

$ git status

and paste the output here?
Does is show any accidental uncommited changes to gammapy/background/ring.py?
What is the commit at the top when you run

$ git log

?

Alternatively ... if you can't track down the errors, could you try this branch in a clean folder?

cd /tmp
git clone https://github.com/JouvinLea/gammapy.git
cd gammapy
git checkout BackgroundModelling
python setup.py test

If these suggestions don't help, we'll do screenshare tomorrow and figure it out together.

Member

cdeil commented Jan 18, 2016

@JouvinLea - I think for some reason your Gammapy working director contains changes unrelated to this PR that are causing the errors you mention.

The last error you mentioned is

NTERNALERROR>   File "gammapy/background/ring.py", line 131
INTERNALERROR>     return (r_out ** 2 - r_in ** 2) / theta ** 2
INTERNALERROR>                                                ^
INTERNALERROR> IndentationError: unindent does not match any outer indentation level

In this branch and Gammapy master there is no IndentationError in this file / function:
https://github.com/gammapy/gammapy/blob/master/gammapy/background/ring.py#L131

Can you try

$ git status

and paste the output here?
Does is show any accidental uncommited changes to gammapy/background/ring.py?
What is the commit at the top when you run

$ git log

?

Alternatively ... if you can't track down the errors, could you try this branch in a clean folder?

cd /tmp
git clone https://github.com/JouvinLea/gammapy.git
cd gammapy
git checkout BackgroundModelling
python setup.py test

If these suggestions don't help, we'll do screenshare tomorrow and figure it out together.

@JouvinLea

This comment has been minimized.

Show comment
Hide comment
@JouvinLea

JouvinLea Jan 19, 2016

Contributor

When I try git status:
On branch BackgroundModelling
Your branch is ahead of 'origin/BackgroundModelling' by 4 commits.
(use "git push" to publish your local commits)

Untracked files:
(use "git add ..." to include in what will be committed)

gammapy/background/#models.py#
gammapy/data/#event_list.py#
gammapy/irf/#effective_area_table.py#

nothing added to commit but untracked files present (use "git add" to track)

But as you said what is weird is that there are no indentation error.
When I tried to clone the branch in a clean folder and run the test, I don’t have any indentationerror anymore but I have these mistakes:
filename = '/Users/jouvin/Desktop/these/test_Gammapy/gammapy-extra/test_datasets/spectrum/spectrum_analysis_example.yaml'
logger = None

def read_yaml(filename, logger=None):
    """
    Read config from YAML file
    """
    import yaml
    if logger is not None:
        logger.info('Reading {}'.format(filename))
  with open(filename) as fh:

E IOError: [Errno 2] No such file or directory: '/Users/jouvin/Desktop/these/test_Gammapy/gammapy-extra/test_datasets/spectrum/spectrum_analysis_example.yaml'

gammapy/utils/scripts.py:149: IOError
=== 7 failed, 287 passed, 47 skipped, 30 xfailed, 8 xpassed in 60.58 seconds ===

Le 18 janv. 2016 à 19:49, Christoph Deil notifications@github.com a écrit :

@JouvinLea - I think for some reason your Gammapy working director contains changes unrelated to this PR that are causing the errors you mention.

The last error you mentioned is

NTERNALERROR> File "gammapy/background/ring.py", line 131
INTERNALERROR> return (r_out ** 2 - r_in ** 2) / theta ** 2
INTERNALERROR> ^
INTERNALERROR> IndentationError: unindent does not match any outer indentation level
In this branch and Gammapy master there is no IndentationError in this file / function:
https://github.com/gammapy/gammapy/blob/master/gammapy/background/ring.py#L131

Can you try

$ git status
and paste the output here?
Does is show any accidental uncommited changes to gammapy/background/ring.py?
What is the commit at the top when you run

$ git log
?

Alternatively ... if you can't track down the errors, could you try this branch in a clean folder?

cd /tmp
git clone https://github.com/JouvinLea/gammapy.git
cd gammapy
git checkout BackgroundModelling
python setup.py test
If these suggestions don't help, we'll do screenshare tomorrow and figure it out together.


Reply to this email directly or view it on GitHub.

Contributor

JouvinLea commented Jan 19, 2016

When I try git status:
On branch BackgroundModelling
Your branch is ahead of 'origin/BackgroundModelling' by 4 commits.
(use "git push" to publish your local commits)

Untracked files:
(use "git add ..." to include in what will be committed)

gammapy/background/#models.py#
gammapy/data/#event_list.py#
gammapy/irf/#effective_area_table.py#

nothing added to commit but untracked files present (use "git add" to track)

But as you said what is weird is that there are no indentation error.
When I tried to clone the branch in a clean folder and run the test, I don’t have any indentationerror anymore but I have these mistakes:
filename = '/Users/jouvin/Desktop/these/test_Gammapy/gammapy-extra/test_datasets/spectrum/spectrum_analysis_example.yaml'
logger = None

def read_yaml(filename, logger=None):
    """
    Read config from YAML file
    """
    import yaml
    if logger is not None:
        logger.info('Reading {}'.format(filename))
  with open(filename) as fh:

E IOError: [Errno 2] No such file or directory: '/Users/jouvin/Desktop/these/test_Gammapy/gammapy-extra/test_datasets/spectrum/spectrum_analysis_example.yaml'

gammapy/utils/scripts.py:149: IOError
=== 7 failed, 287 passed, 47 skipped, 30 xfailed, 8 xpassed in 60.58 seconds ===

Le 18 janv. 2016 à 19:49, Christoph Deil notifications@github.com a écrit :

@JouvinLea - I think for some reason your Gammapy working director contains changes unrelated to this PR that are causing the errors you mention.

The last error you mentioned is

NTERNALERROR> File "gammapy/background/ring.py", line 131
INTERNALERROR> return (r_out ** 2 - r_in ** 2) / theta ** 2
INTERNALERROR> ^
INTERNALERROR> IndentationError: unindent does not match any outer indentation level
In this branch and Gammapy master there is no IndentationError in this file / function:
https://github.com/gammapy/gammapy/blob/master/gammapy/background/ring.py#L131

Can you try

$ git status
and paste the output here?
Does is show any accidental uncommited changes to gammapy/background/ring.py?
What is the commit at the top when you run

$ git log
?

Alternatively ... if you can't track down the errors, could you try this branch in a clean folder?

cd /tmp
git clone https://github.com/JouvinLea/gammapy.git
cd gammapy
git checkout BackgroundModelling
python setup.py test
If these suggestions don't help, we'll do screenshare tomorrow and figure it out together.


Reply to this email directly or view it on GitHub.

@JouvinLea

This comment has been minimized.

Show comment
Hide comment
@JouvinLea

JouvinLea Jan 19, 2016

Contributor

I did a git fetch upstream in order to have the latest version of the gammapy master branch.
Then in my local branch BackgroundModelling I did a git rebase remotes/upstream/master. There was no conflict to solve but still the same issue...
Le 19 janv. 2016 à 09:28, Lea Jouvin ljouvin@apc.in2p3.fr a écrit :

When I try git status:
On branch BackgroundModelling
Your branch is ahead of 'origin/BackgroundModelling' by 4 commits.
(use "git push" to publish your local commits)

Untracked files:
(use "git add ..." to include in what will be committed)

gammapy/background/#models.py#
gammapy/data/#event_list.py#
gammapy/irf/#effective_area_table.py#

nothing added to commit but untracked files present (use "git add" to track)

But as you said what is weird is that there are no indentation error.
When I tried to clone the branch in a clean folder and run the test, I don’t have any indentationerror anymore but I have these mistakes:
filename = '/Users/jouvin/Desktop/these/test_Gammapy/gammapy-extra/test_datasets/spectrum/spectrum_analysis_example.yaml'
logger = None

def read_yaml(filename, logger=None):
    """
    Read config from YAML file
    """
    import yaml
    if logger is not None:
        logger.info('Reading {}'.format(filename))
  with open(filename) as fh:

E IOError: [Errno 2] No such file or directory: '/Users/jouvin/Desktop/these/test_Gammapy/gammapy-extra/test_datasets/spectrum/spectrum_analysis_example.yaml'

gammapy/utils/scripts.py:149: IOError
=== 7 failed, 287 passed, 47 skipped, 30 xfailed, 8 xpassed in 60.58 seconds ===

Le 18 janv. 2016 à 19:49, Christoph Deil notifications@github.com a écrit :

@JouvinLea - I think for some reason your Gammapy working director contains changes unrelated to this PR that are causing the errors you mention.

The last error you mentioned is

NTERNALERROR> File "gammapy/background/ring.py", line 131
INTERNALERROR> return (r_out ** 2 - r_in ** 2) / theta ** 2
INTERNALERROR> ^
INTERNALERROR> IndentationError: unindent does not match any outer indentation level
In this branch and Gammapy master there is no IndentationError in this file / function:
https://github.com/gammapy/gammapy/blob/master/gammapy/background/ring.py#L131

Can you try

$ git status
and paste the output here?
Does is show any accidental uncommited changes to gammapy/background/ring.py?
What is the commit at the top when you run

$ git log
?

Alternatively ... if you can't track down the errors, could you try this branch in a clean folder?

cd /tmp
git clone https://github.com/JouvinLea/gammapy.git
cd gammapy
git checkout BackgroundModelling
python setup.py test
If these suggestions don't help, we'll do screenshare tomorrow and figure it out together.


Reply to this email directly or view it on GitHub.

Contributor

JouvinLea commented Jan 19, 2016

I did a git fetch upstream in order to have the latest version of the gammapy master branch.
Then in my local branch BackgroundModelling I did a git rebase remotes/upstream/master. There was no conflict to solve but still the same issue...
Le 19 janv. 2016 à 09:28, Lea Jouvin ljouvin@apc.in2p3.fr a écrit :

When I try git status:
On branch BackgroundModelling
Your branch is ahead of 'origin/BackgroundModelling' by 4 commits.
(use "git push" to publish your local commits)

Untracked files:
(use "git add ..." to include in what will be committed)

gammapy/background/#models.py#
gammapy/data/#event_list.py#
gammapy/irf/#effective_area_table.py#

nothing added to commit but untracked files present (use "git add" to track)

But as you said what is weird is that there are no indentation error.
When I tried to clone the branch in a clean folder and run the test, I don’t have any indentationerror anymore but I have these mistakes:
filename = '/Users/jouvin/Desktop/these/test_Gammapy/gammapy-extra/test_datasets/spectrum/spectrum_analysis_example.yaml'
logger = None

def read_yaml(filename, logger=None):
    """
    Read config from YAML file
    """
    import yaml
    if logger is not None:
        logger.info('Reading {}'.format(filename))
  with open(filename) as fh:

E IOError: [Errno 2] No such file or directory: '/Users/jouvin/Desktop/these/test_Gammapy/gammapy-extra/test_datasets/spectrum/spectrum_analysis_example.yaml'

gammapy/utils/scripts.py:149: IOError
=== 7 failed, 287 passed, 47 skipped, 30 xfailed, 8 xpassed in 60.58 seconds ===

Le 18 janv. 2016 à 19:49, Christoph Deil notifications@github.com a écrit :

@JouvinLea - I think for some reason your Gammapy working director contains changes unrelated to this PR that are causing the errors you mention.

The last error you mentioned is

NTERNALERROR> File "gammapy/background/ring.py", line 131
INTERNALERROR> return (r_out ** 2 - r_in ** 2) / theta ** 2
INTERNALERROR> ^
INTERNALERROR> IndentationError: unindent does not match any outer indentation level
In this branch and Gammapy master there is no IndentationError in this file / function:
https://github.com/gammapy/gammapy/blob/master/gammapy/background/ring.py#L131

Can you try

$ git status
and paste the output here?
Does is show any accidental uncommited changes to gammapy/background/ring.py?
What is the commit at the top when you run

$ git log
?

Alternatively ... if you can't track down the errors, could you try this branch in a clean folder?

cd /tmp
git clone https://github.com/JouvinLea/gammapy.git
cd gammapy
git checkout BackgroundModelling
python setup.py test
If these suggestions don't help, we'll do screenshare tomorrow and figure it out together.


Reply to this email directly or view it on GitHub.

@cdeil

This comment has been minimized.

Show comment
Hide comment
@cdeil

cdeil Jan 19, 2016

Member

In your git status output I see this:

Your branch is ahead of 'origin/BackgroundModelling' by 4 commits.

So it looks like you have made four commits locally, which you haven't pushed to Github.
It looks like you edited (probably accidentally) gammapy/background/ring.py, introducing that IndentationError and then committed it locally.

To confirm, can you please show the topmost commit from

git log

here?

And you could run

git push origin BackgroundModelling

to push those four commits here, so that I (and you) can see them on the Github web interface?

Member

cdeil commented Jan 19, 2016

In your git status output I see this:

Your branch is ahead of 'origin/BackgroundModelling' by 4 commits.

So it looks like you have made four commits locally, which you haven't pushed to Github.
It looks like you edited (probably accidentally) gammapy/background/ring.py, introducing that IndentationError and then committed it locally.

To confirm, can you please show the topmost commit from

git log

here?

And you could run

git push origin BackgroundModelling

to push those four commits here, so that I (and you) can see them on the Github web interface?

@cdeil

This comment has been minimized.

Show comment
Hide comment
@cdeil

cdeil Jan 19, 2016

Member

One more thing ... if you want a GUI for git, you can use the PyCharm IDE or use an extra program like https://desktop.github.com/ or https://git-scm.com/docs/gitk .
So far I'm using the command line for git, in combination with the Github web interface, but I think those other options are also pretty nice ..

Member

cdeil commented Jan 19, 2016

One more thing ... if you want a GUI for git, you can use the PyCharm IDE or use an extra program like https://desktop.github.com/ or https://git-scm.com/docs/gitk .
So far I'm using the command line for git, in combination with the Github web interface, but I think those other options are also pretty nice ..

@JouvinLea

This comment has been minimized.

Show comment
Hide comment
@JouvinLea

JouvinLea Jan 19, 2016

Contributor

commit 5a0e63b
Author: Lea Jouvin lea.jouvin@gmail.com
Date: Mon Jan 18 17:52:24 2016 +0100

t

commit fde3b4e
Author: Lea Jouvin lea.jouvin@gmail.com
Date: Mon Jan 18 17:51:52 2016 +0100

defining the testing function

Le 19 janv. 2016 à 10:02, Christoph Deil notifications@github.com a écrit :

One more thing ... if you want a GUI for git, you can use the PyCharm IDE or use an extra program like https://desktop.github.com/ or https://git-scm.com/docs/gitk .
So far I'm using the command line for git, in combination with the Github web interface, but I think those other options are also pretty nice ..


Reply to this email directly or view it on GitHub.

Contributor

JouvinLea commented Jan 19, 2016

commit 5a0e63b
Author: Lea Jouvin lea.jouvin@gmail.com
Date: Mon Jan 18 17:52:24 2016 +0100

t

commit fde3b4e
Author: Lea Jouvin lea.jouvin@gmail.com
Date: Mon Jan 18 17:51:52 2016 +0100

defining the testing function

Le 19 janv. 2016 à 10:02, Christoph Deil notifications@github.com a écrit :

One more thing ... if you want a GUI for git, you can use the PyCharm IDE or use an extra program like https://desktop.github.com/ or https://git-scm.com/docs/gitk .
So far I'm using the command line for git, in combination with the Github web interface, but I think those other options are also pretty nice ..


Reply to this email directly or view it on GitHub.

@JouvinLea

This comment has been minimized.

Show comment
Hide comment
@JouvinLea

JouvinLea Jan 19, 2016

Contributor

When I run git push origin BackgroundModelling, it says Everything up-to-date!

Le 19 janv. 2016 à 10:08, Lea Jouvin ljouvin@apc.in2p3.fr a écrit :

commit 5a0e63b
Author: Lea Jouvin lea.jouvin@gmail.com
Date: Mon Jan 18 17:52:24 2016 +0100

t

commit fde3b4e
Author: Lea Jouvin lea.jouvin@gmail.com
Date: Mon Jan 18 17:51:52 2016 +0100

defining the testing function

Le 19 janv. 2016 à 10:02, Christoph Deil notifications@github.com a écrit :

One more thing ... if you want a GUI for git, you can use the PyCharm IDE or use an extra program like https://desktop.github.com/ or https://git-scm.com/docs/gitk .
So far I'm using the command line for git, in combination with the Github web interface, but I think those other options are also pretty nice ..


Reply to this email directly or view it on GitHub.

Contributor

JouvinLea commented Jan 19, 2016

When I run git push origin BackgroundModelling, it says Everything up-to-date!

Le 19 janv. 2016 à 10:08, Lea Jouvin ljouvin@apc.in2p3.fr a écrit :

commit 5a0e63b
Author: Lea Jouvin lea.jouvin@gmail.com
Date: Mon Jan 18 17:52:24 2016 +0100

t

commit fde3b4e
Author: Lea Jouvin lea.jouvin@gmail.com
Date: Mon Jan 18 17:51:52 2016 +0100

defining the testing function

Le 19 janv. 2016 à 10:02, Christoph Deil notifications@github.com a écrit :

One more thing ... if you want a GUI for git, you can use the PyCharm IDE or use an extra program like https://desktop.github.com/ or https://git-scm.com/docs/gitk .
So far I'm using the command line for git, in combination with the Github web interface, but I think those other options are also pretty nice ..


Reply to this email directly or view it on GitHub.

@cdeil

This comment has been minimized.

Show comment
Hide comment
@cdeil

cdeil Jan 19, 2016

Member

I now the the cause of the IndentationError in the diff of this PR on Github:
https://github.com/gammapy/gammapy/pull/383/files#diff-4398aa19c956719cfc6098402e1ad301

The docstring in that function is indented by 4 spaces as it should be.
But the code on line 131 is only indented by 3 spaces.

You introduced this in the last commit here:
JouvinLea@5a0e63b
https://github.com/gammapy/gammapy/pull/383/commits

To get rid of this commit locally, you can use these commands:

git checkout BackgroundModelling
git reset --hard fde3b4

and then to push that version of the branch to Github:

git push -f origin BackgroundModelling

Does this resolve the issue for you, i.e. do tests then pass if you run this?

python setup.py test -V
Member

cdeil commented Jan 19, 2016

I now the the cause of the IndentationError in the diff of this PR on Github:
https://github.com/gammapy/gammapy/pull/383/files#diff-4398aa19c956719cfc6098402e1ad301

The docstring in that function is indented by 4 spaces as it should be.
But the code on line 131 is only indented by 3 spaces.

You introduced this in the last commit here:
JouvinLea@5a0e63b
https://github.com/gammapy/gammapy/pull/383/commits

To get rid of this commit locally, you can use these commands:

git checkout BackgroundModelling
git reset --hard fde3b4

and then to push that version of the branch to Github:

git push -f origin BackgroundModelling

Does this resolve the issue for you, i.e. do tests then pass if you run this?

python setup.py test -V
Lea Jouvin

@cdeil cdeil referenced this pull request Jan 25, 2016

Merged

Add EventList plots #415

Lea Jouvin added some commits Jan 25, 2016

Lea Jouvin
Lea Jouvin
@cdeil

This comment has been minimized.

Show comment
Hide comment
@cdeil

cdeil Feb 1, 2016

Member

@JouvinLea - You have to leave a comment on a PR if you want me to review or merge it.

I forgot about this one and now see that it's almost ready. Sorry!
I'll leave a few inline comments now and once those are addressed this is ready to be merged.

Member

cdeil commented Feb 1, 2016

@JouvinLea - You have to leave a comment on a PR if you want me to review or merge it.

I forgot about this one and now see that it's almost ready. Sorry!
I'll leave a few inline comments now and once those are addressed this is ready to be merged.

Show outdated Hide outdated gammapy/data/tests/test_event_list.py Outdated
Show outdated Hide outdated gammapy/data/tests/test_data_store.py Outdated

Lea Jouvin added some commits Feb 1, 2016

@JouvinLea

This comment has been minimized.

Show comment
Hide comment
@JouvinLea

JouvinLea Feb 1, 2016

Contributor

I think we can merge it now!

Contributor

JouvinLea commented Feb 1, 2016

I think we can merge it now!

Show outdated Hide outdated gammapy/data/tests/test_event_list.py Outdated
Lea Jouvin
Lea Jouvin
@cdeil

This comment has been minimized.

Show comment
Hide comment
@cdeil

cdeil Feb 1, 2016

Member

I've left 2 or 3 nitpicky inline comments.
The main thing is to make the test pass, once travis-ci gives green light I'll merge this PR!

Member

cdeil commented Feb 1, 2016

I've left 2 or 3 nitpicky inline comments.
The main thing is to make the test pass, once travis-ci gives green light I'll merge this PR!

Lea Jouvin
@JouvinLea

This comment has been minimized.

Show comment
Hide comment
@JouvinLea

JouvinLea Feb 1, 2016

Contributor

ok I think I take into acount all your suggestions!
Le 1 févr. 2016 à 17:58, Christoph Deil notifications@github.com a écrit :

I've left 2 or 3 nitpicky inline comments.
The main thing is to make the test pass, once travis-ci gives green light I'll merge this PR!


Reply to this email directly or view it on GitHub.

Contributor

JouvinLea commented Feb 1, 2016

ok I think I take into acount all your suggestions!
Le 1 févr. 2016 à 17:58, Christoph Deil notifications@github.com a écrit :

I've left 2 or 3 nitpicky inline comments.
The main thing is to make the test pass, once travis-ci gives green light I'll merge this PR!


Reply to this email directly or view it on GitHub.

@cdeil

This comment has been minimized.

Show comment
Hide comment
@cdeil

cdeil Feb 1, 2016

Member

Merging this now. 🎉
@JouvinLea Thanks for your patience with this PR!

Member

cdeil commented Feb 1, 2016

Merging this now. 🎉
@JouvinLea Thanks for your patience with this PR!

cdeil added a commit that referenced this pull request Feb 1, 2016

@cdeil cdeil merged commit 38a5e5b into gammapy:master Feb 1, 2016

0 of 2 checks passed

continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
@cdeil

This comment has been minimized.

Show comment
Hide comment
@cdeil

cdeil Feb 1, 2016

Member

@JouvinLea - I've added a changelog entry for this and added you to the contributor's list for Gammapy in this commit in master: 96c3cbf

The docs for the class are now online:http://gammapy.readthedocs.org/en/latest/api/gammapy.background.EnergyOffsetArray.html

There's more to do for this class.
I think these are the main extra methods that need to be written:

  • read / write to FITS
  • extract counts vector at given offset
  • fill 3D cube by rotating.

@JouvinLea - I probably won't have time for this in the coming weeks. And I guess you also want to focus on getting observation handling and spectra working. Just in case you do have time to work on this, let me know and I can give some pointers how these methods should be implemented, so that you don't waste time.

Member

cdeil commented Feb 1, 2016

@JouvinLea - I've added a changelog entry for this and added you to the contributor's list for Gammapy in this commit in master: 96c3cbf

The docs for the class are now online:http://gammapy.readthedocs.org/en/latest/api/gammapy.background.EnergyOffsetArray.html

There's more to do for this class.
I think these are the main extra methods that need to be written:

  • read / write to FITS
  • extract counts vector at given offset
  • fill 3D cube by rotating.

@JouvinLea - I probably won't have time for this in the coming weeks. And I guess you also want to focus on getting observation handling and spectra working. Just in case you do have time to work on this, let me know and I can give some pointers how these methods should be implemented, so that you don't waste time.

@JouvinLea

This comment has been minimized.

Show comment
Hide comment
@JouvinLea

JouvinLea Feb 1, 2016

Contributor

Depending on how I work on the grouping and the PSF perhaps I will work a little bit on this too. I think it could be interesting to work on that when I am in Heidelberg. This is always better if I have time to start to think on the implementation before coming. I am not here from Wednesday to Sunday this week and I will not work on this class tomorrow.
If you have time to give me some pointers, it could be interesting!!
Le 1 févr. 2016 à 18:35, Christoph Deil notifications@github.com a écrit :

@JouvinLea - I've added a changelog entry for this and added you to the contributor's list for Gammapy in this commit in master: 96c3cbf

The docs for the class are now online:http://gammapy.readthedocs.org/en/latest/api/gammapy.background.EnergyOffsetArray.html

There's more to do for this class.
I think these are the main extra methods that need to be written:

read / write to FITS
extract counts vector at given offset
fill 3D cube by rotating.
@JouvinLea - I probably won't have time for this in the coming weeks. And I guess you also want to focus on getting observation handling and spectra working. Just in case you do have time to work on this, let me know and I can give some pointers how these methods should be implemented, so that you don't waste time.


Reply to this email directly or view it on GitHub.

Contributor

JouvinLea commented Feb 1, 2016

Depending on how I work on the grouping and the PSF perhaps I will work a little bit on this too. I think it could be interesting to work on that when I am in Heidelberg. This is always better if I have time to start to think on the implementation before coming. I am not here from Wednesday to Sunday this week and I will not work on this class tomorrow.
If you have time to give me some pointers, it could be interesting!!
Le 1 févr. 2016 à 18:35, Christoph Deil notifications@github.com a écrit :

@JouvinLea - I've added a changelog entry for this and added you to the contributor's list for Gammapy in this commit in master: 96c3cbf

The docs for the class are now online:http://gammapy.readthedocs.org/en/latest/api/gammapy.background.EnergyOffsetArray.html

There's more to do for this class.
I think these are the main extra methods that need to be written:

read / write to FITS
extract counts vector at given offset
fill 3D cube by rotating.
@JouvinLea - I probably won't have time for this in the coming weeks. And I guess you also want to focus on getting observation handling and spectra working. Just in case you do have time to work on this, let me know and I can give some pointers how these methods should be implemented, so that you don't waste time.


Reply to this email directly or view it on GitHub.

@joleroi

This comment has been minimized.

Show comment
Hide comment
@joleroi

joleroi Mar 16, 2016

Contributor

@JouvinLea @cdeil
What is the status of the EnergyOffsetArray. Can I use it for #490? I have in mind that you planned to write a generic base class. But from
http://gammapy.readthedocs.org/en/latest/background/energy_offset_array.html?highlight=energy%20offset%20array
it sound more like something coupled to events.

Contributor

joleroi commented Mar 16, 2016

@JouvinLea @cdeil
What is the status of the EnergyOffsetArray. Can I use it for #490? I have in mind that you planned to write a generic base class. But from
http://gammapy.readthedocs.org/en/latest/background/energy_offset_array.html?highlight=energy%20offset%20array
it sound more like something coupled to events.

@cdeil

This comment has been minimized.

Show comment
Hide comment
@cdeil

cdeil Mar 16, 2016

Member

@joleroi - I'll come talk to you about this.

Member

cdeil commented Mar 16, 2016

@joleroi - I'll come talk to you about this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment