search

jobovy / apogee

Tools for dealing with APOGEE data
BSD 3-Clause "New" or "Revised" License
43 stars 25 forks source link

Fitsio optional #49

Closed npricejones closed 6 years ago

npricejones commented 6 years ago

Fixed syntax error so fitsio no longer required.

jobovy commented 6 years ago

Thanks! Does this cover all instances of using fitsio? I think there might be a few more: https://github.com/jobovy/apogee/commit/6ed096bf8cc5f7f097cce7ce11267f0a4357acda#diff-4635588c6e5966367f42330f234ff026

npricejones commented 6 years ago

I'll track down the rest of those instances.

jobovy commented 6 years ago

Great, let me know when it's ready to look at again.

npricejones commented 6 years ago

I got all of the instances of fitsio read or write. While I'm doing this, can I suggest changing fitsio -> astropy in setup.py since the latter is typically much easier to install than the former? Or would you prefer to return the fitsio dependency?

jobovy commented 6 years ago

Yes, let's change fitsio to astropy in setup.py; then people can install fitsio and use it if they really want to (like me!)

npricejones commented 6 years ago

I've changed the setup and README files to use astropy as the default dependency for FITs and indicated that separately installing fitsio will cause it to take precedence over astropy

jobovy commented 6 years ago

Thanks, will merge!