caracal-pipeline / caracal

Containerized Automated Radio Astronomy Calibration (CARACal) pipeline
GNU General Public License v2.0
28 stars 6 forks source link

Too many unnecessary dependencies #1490

Open SpheMakh opened 1 year ago

SpheMakh commented 1 year ago

There are too many abuses of the utils package. These are not meant to be substitutes for things that should be packaged separately like flag_Uzeros and image_contsub.

Having these scripts in utils requires things like astropy and regions which can have finicky dependency trees which make the pipeline difficult to build and test in a robust way.

If something has to added in utils it should not need extra dependencies, instead, we should do something like this:

https://github.com/caracal-pipeline/caracal/blob/b1975b9c80f2253ba1ddceca79ae89f57a5ad127/caracal/workers/utils/manage_flagsets.py#L150-L181

SpheMakh commented 1 year ago

This fix also has to be reverted and issue #1280 addressed without adding python-casacore as dependency.

SpheMakh commented 1 year ago

I added scipy, astropy, regions and astroquerry as optional dependencies because so installing caracal in the standard way will not have full functionality until the relevant functions/workers containerize the tasks.

This is the error message you get if the run needs an optional dependency

2023-04-14 18:34:44 CARACal ERROR: Pipeline run requires optional dependencies, please re-install caracal as: 
 'pip install caracal[all]' or, install the missing package as: 
 'pip install caracal[astropy]' [ExtraDependencyError]
2023-04-14 18:34:44 CARACal INFO:   More information can be found in the logfile at testout/logs-20230414-183440/log-caracal.txt
2023-04-14 18:34:44 CARACal INFO: exiting with error code 1

If you know the optional dependency you need, you can install using

pip install caracal[<foo>] # foo = scipy | astropy | astroquerry | regions

or, install with all optional dependencies using

pip install caracal[all]

I left python-casacore, nbconvert, radiopadre, and related since necessary functionality is tightly coupled to these tools. It will require a lot more time to migrate these.

It's good for us to remember that caracal has to be light and uncomplicated; the heavy-duty (and often complicated to build stuff) should be isolated in containers; at least in its simplest form.

I also ran autopep8 to make the code a bit more readable and easier to debug. The PR is passing all the tests, but maybe still worth manually testing some functionality, in particular the ddcal and line workers and image_contsub and flag_uzeros since this is where the bulk of the changes were made.

These changes are in PR #1498 in branch crystal-model-chunks