search

MWATelescope / mwa_pb

MWA Primary Beam model code
MIT License
2 stars 8 forks source link

install requires in setup.py needs updating #4

Closed bhazelton closed 4 years ago

bhazelton commented 5 years ago

Pillow doesn't appear anywhere else in the repo.

Strictly speaking, none of the things listed in install_requires are required to import the module because the __init__.py doesn't import anything.

I wonder if it would be reasonable to take ephem out of install requires and list it in extras_require since it's only needed in a few places. I should point out that ephem is no longer maintained and doesn't handle leap seconds properly, so moving to astropy is advisable.

I'd like to import this library in another library, so limiting the number of dependencies would be helpful (we already depend on numpy, scipy and astropy, so those don't bother me).

andreww5au commented 5 years ago

Hi Bryna, Pillow is imported as PIL (it replaces the old, unmaintained PIL library), and is used in mwa_pb/skymap.py. I can move that and ephem into extras_require.

I'm in the process of porting all uses of 'ephem' to 'skyfield' instead, plus as much of the astropy code as I can. Using skyfield to convert alt/az to ra/dec is much faster than astropy, so much so that the code in suppress.py, for example, runs _fourtimes faster using skyfield, because with astropy it spent 75% of the execution time in a few calls to SkyCoord.transform_to().

bhazelton commented 5 years ago

Great, thank you! Sounds like I should look harder at Skyfield. I've stayed away from it because it's a single developer rather than a whole community. Have you tested its results vs astropy?

andreww5au commented 4 years ago

Hi Bryna, skyfield and astropy differ by 10-20 arcseconds in altaz<->radec conversions, but every other code I can find to do that differs with both astropy and skyfield by that much or more. I think it's a case of differing assumptions (calculating geocentric vs geodetic zenith for the altaz frame, for example), but I don't know which is 'correct'. The MWA beam size is a few orders of magnitude bigger then the errors, so I went with skyfield for the speed.

I've just merged in changes that mean mwa_pb now supports Python 3, uses skyfield instead of astropy for a lot of the altaz/radec conversions, and moves 'ephem' and 'Pillow' into 'extras_require'. The 'ephem' library is now only used once in mwa_pb/skymap.py, to determine the name of the constellation that a given ra/dec is in.