Use classmethods for map factory functions

[cdeil]

I saw that you have these four functions that generate map objects: http://fermipy.readthedocs.io/en/latest/_modules/fermipy/skymap.html#make_coadd_hpx http://fermipy.readthedocs.io/en/latest/_modules/fermipy/skymap.html#make_coadd_map http://fermipy.readthedocs.io/en/latest/_modules/fermipy/skymap.html#make_coadd_wcs http://fermipy.readthedocs.io/en/latest/_modules/fermipy/skymap.html#read_map_from_fits

They don't have a docstring, but I gather this is functionality that is part of the public API that is useful to have?

If so, I'd suggest to change them to be classmethods (see explanation e.g. here).

Grouping such functions that generate objects of a given class as classmethods on the class usually makes them easier to find and a little easier to use (no extra import).

It's also the pattern that's used for many things in Astropy and Gammapy, see e.g. https://gammapy.readthedocs.io/en/latest/_modules/gammapy/image/maps.html#SkyMap.read https://gammapy.readthedocs.io/en/latest/_modules/gammapy/image/maps.html#SkyMap.empty

@woodmd - Thoughts?

[woodmd]

I haven't really thought that carefully about which methods should be part of the public interface but I think these would probably fall under the category of functionality that should be public. Do the gammapy wcs classes have something equivalent to a coadd method?

Regarding the use of factory methods, I've also used this pattern elsewhere so making these functions into classmethods would be fine with me.

[cdeil]

We have SkyMap.reproject and SkyCube.reproject_to.

For coadding we have MosaicImage.make_images and ad-hoc implementations in some scripts. This is a weird API and needs to be improved. I think I like the total_map = SkyMap.coadd(maps) API that does reproject and add, and maybe total_map = SkyMap.stack(maps) that checks that WCSs are identical for the maps and just sums.

I'm not sure how exactly to proceed, i.e. where to put my development effort. Improve Fermi classes now and add tests, then move to Gammapy? Or improve the Gammapy classes now and move the HPX classes now, then improve them in Gammapy and once it's done, remove the ones from Fermipy and introduce the dependency on Gammapy?

@woodmd - Would you be available for e short Skype call this week or next?

[woodmd]

Since our plan was to eventually merge the existing functionality in fermipy into gammapy maybe the effort would be better spent to develop functionality/tests in gammapy. At present we're not really doing any active work on the HPX classes so I think moving the development of those into gammapy wouldn't be a big hindrance assuming that we can having something working on a ~month timescale (i.e. something that reproduces the functionality we already had in fermipy).

Your proposal for coadd and stack methods sounds reasonable to me. I think once these are available we could probably already switch fermipy over to using gammapy for WCS-based maps.

Regarding a call, sometime early next week would be fine. I'm generally available anytime after 8AM PDT/5PM CET.

[woodmd]

This issue is now obsolete since we've undertaken to migrate fermipy maps classes to gammapy.maps. There we've tried to follow gammapy/astropy patterns for factory methods.