search

NASA-Planetary-Science / sbpy

A Python package for small bodies research
https://sbpy.org/
Other
66 stars 34 forks source link

Conda installability for M1 mac #382

Closed drewoldag closed 11 months ago

drewoldag commented 11 months ago

High-level problem description Currently it doesn't seem possible to install sbpy with conda on an M1 mac because there is no conda-forge distribution for synphot for M1 mac.

Is it possible to make synphot an optional dependency for sbpy?

I've created an issue on synphot (https://github.com/spacetelescope/synphot_refactor/issues/358) asking for a noarch or M1 release, but it's not clear if that is something the maintainers will have time to do or possible.

I looked through the sbpy code and while synphot is mentioned in many places, it does seem like perhaps it's an optional dependency. I'm basing that code snippets like this https://github.com/NASA-Planetary-Science/sbpy/blob/main/sbpy/calib/core.py#L139

However, I don't have the subject matter expertise to say for sure that whether it's optional or not :)

mkelley commented 11 months ago

Looks like when I made synphot required, I didn't clean up the codebase in this respect. I suppose it would be possible to move it back to optional, but this would disable a lot of the photometric and spectroscopic capabilities, and will take some work to sort out the testing suite. In what way is sbpy needed for your installation?

drewoldag commented 11 months ago

sbpy is being used in this project: https://github.com/dirac-institute/sorcha

From what I can tell, we make use of the phase functions that are defined in sbpy.photometry. i.e. from sbpy.photometry import HG, HG1G2, HG12_Pen16, LinearPhaseFunc https://github.com/dirac-institute/sorcha/blob/main/src/sorcha/modules/PPCalculateApparentMagnitudeInFilter.py#L4

I'm not sure if that's answering your question or not though :)

jianyangli commented 11 months ago

synphot is only used to convert between magnitude and reflectance in those classes. If you don't need those functions, then synphot can be optional.

Mike, personally I think synphot can be optional. sbpy provides a wide range of applications, of which quite a fraction doesn't really need synphot, such as this case and data submodule, and those to be implemented in the future including obsutil, shape, thermal, imageanalysis. I understand it'll be a lot of work to sort it out, but probably worth doing to expand the applicability of sbpy in the long term.

mkelley commented 11 months ago

OK, this sounds reasonable and I will give it a go.

drewoldag commented 11 months ago

I received word that synphot now has a M1 build available on conda-forge

I haven't had the chance to confirm that the installation works as expected, but I did check the list of files available on conda-forge and I see osx arm builds listed. So at this point, I would argue that it make sense to stop work on trying to make synphot an optional dependency, especially if it seems like it's going to be a difficult process.

mkelley commented 11 months ago

@drewoldag Thanks for the update. I'll still proceed with making this package optional, but will close this issue. Please re-open if the original issue with M1 installation persists.