search

threeML / hawc_hal

HAWC Accelerated Likelihood - python-only framework for HAWC data analysis
BSD 3-Clause "New" or "Revised" License
11 stars 21 forks source link

Update map_tree.py #56

Closed hayalaso closed 3 years ago

hayalaso commented 3 years ago

Moving the roimap array as a table instead of being in the header when using HealpixMapROI.

The current patch can write hdf5 files now. Reading the HDF5 using the script hal_hdf5_to_fits.py gives a bit of problems since hal expects to read the roimap from the header. I'll see if this can be modified somehow.

hayalaso commented 3 years ago

I tested the changes using both HealpixROIMap and HealpixConeMap. I also used the script hal_hdf5_to_fits.py and it seems it works for both of them. I also noticed, although probably not that important, that tables 3.5.2 or bigger was needed. Maybe not necessary in py3.

henrikef commented 3 years ago

@hayalaso is this ready for review or are you still working on it?

Could you add something to the tests as well?

hayalaso commented 3 years ago

I got busy with other things the past couple of weeks so I don't remember. I'll need to double-check.

henrikef commented 3 years ago

Btw, on github, using WIP in the pull request name doesn't do anything. Instead, you can mark pull requests as draft: https://github.blog/2019-02-14-introducing-draft-pull-requests/ You can also convert to draft later, there's a link on the upper right, underneath where you can select reviewers.

hayalaso commented 3 years ago

I added a test to write a model map using the HealpixROIMap and also read it back. I think it should be ready to merge.

henrikef commented 3 years ago

Hi @hayalaso!

Thank you for implementing this!

Please pull in the most recent version of the hawc_hal master into your branch and resolve any conflicts. Note that the tests directory moved to hawc_hal/tests.

Once the tests pass, this is ready to merge.

henrikef commented 3 years ago

Fixes #22

Fixes #55

Hm. This isn't working for some reason. Maybe the author has to do it? Might need to close those issues manually.