Change request #2670
Support ENERG colname for energy dispersion fits file
| Status: | Closed | Start date: | 09/04/2018 | |
|---|---|---|---|---|
| Priority: | Normal | Due date: | ||
| Assigned To: |  Knödlseder Jürgen | % Done: | 100% | |
| Category: | - | |||
| Target version: | 1.6.0 | |||
| Duration: |
Description
Right now, GCTAEdisp2D initializes its energy axis with the hardcoded name “ETRUE”:m_inx_etrue = m_edisp.axis("ETRUE")
However, specifications for gamma-ray data formats suggest “ENERG”:
https://gamma-astro-data-formats.readthedocs.io/en/latest/irfs/full_enclosure/edisp/index.html#edisp-2d
Maybe this class should be changed to support multiple column names.
Recurrence
No recurrence.
History
#1
Updated by Knödlseder Jürgen over 5 years ago
- Status changed from New to Pull request
- Assigned To set to Knödlseder Jürgen
- Target version set to 1.6.0
- % Done changed from 0 to 100
I recall that we had at some point a discussion about names of energy columns and wanted to have a distinction between true and reconstructed energies in the column names, but I could not find any trace of that discussion.
Anyway, I changed the code so that both ETRUE and ENERG column names are supported. Code is merged into devel (you may check if everything is okay since I have not made an explicit code check).
#2
Updated by Knödlseder Jürgen over 5 years ago
- Status changed from Pull request to Closed
I also changed the column name in csroot2caldb so that newly generated IRFs now have ENERG as column name.
Code is merged into devel.
#3
Updated by Specovius Andreas over 5 years ago
Knödlseder Jürgen wrote:
I recall that we had at some point a discussion about names of energy columns and wanted to have a distinction between true and reconstructed energies in the column names, but I could not find any trace of that discussion.
I see your point and aggree with you. I don’t know, maybe the suggested data format specs are deprecated?
#4
Updated by Specovius Andreas over 5 years ago
Knödlseder Jürgen wrote:
Anyway, I changed the code so that both
ETRUEandENERGcolumn names are supported. Code is merged intodevel(you may check if everything is okay since I have not made an explicit code check).
Unfortunately in the code you search for the table ENERG which is an axis. An exception is raised. Line1339> if (m_edisp.has_table("ENERG")) {
should be replaced with1339> if (m_edisp.has_axis("ENERG")) {
#5
Updated by Knödlseder Jürgen over 5 years ago
- Status changed from Closed to Feedback
Sorry for that, is corrected now. Can you check again?
#6
Updated by Specovius Andreas over 5 years ago
Knödlseder Jürgen wrote:
Sorry for that, is corrected now. Can you check again?
The updated implementation works correctly
#7
Updated by Knödlseder Jürgen over 5 years ago
- Status changed from Feedback to Closed
Great, thanks.