Cleanup for chunked imaging logic

[thomaswilliamsastro]

Fixes #303
Fixes #272
FIxes #274

The chunkedImaging logic is currently pretty wonky, and this fixes that. Now, we have a couple of cases:

1. You set chunk_num to None.

Previously, this would loop over each stage, combine a cube, remove old cubes and essentially reset the progress at each stage.

Now, this will get the total number of chunks and loop over them in turn, before potentially recombining cubes at the end (in the temp directory), potentially cleaning up and then moving files back. This significantly simplifies how you can interface with running the imaging, since now everything can be done in one call. Just set do_recombine_cubes and do_cleanup (which replaces remove_chunks for consistency) to True in the run_imaging call.

2. You set chunk_num to an integer.

Previously, this way could require re-instantiating the handler, which is awkward. Now, it doesn't do that.

Other small fixes that helped along the way here:

• Fixes an issue where the temp directory would end up under a "None" folder, which was introduced in the previous patch
• Fixes log messages so the chunk numbers are now correct
• Includes some more robust log messages for file copying and moving etc
• More militant about moving back and forth between directories, just in case
• The move at the end has been updated to not include the MS if it gets copied into the temp directory. This would throw up a warning and can be slow!

This has been tested for both the loop over chunks and chunk_num=None case, using/not using a temporary directory (and when using temporary directory, copying vs not copying over the MS). It all works how I'd expect it to (finally)

[low-sky]

I'm testing this PR in the cluster environment. Just noting that the default seems to have moved everything out of temp directories after completion (good) but then looks for the files in those temp directories in the gathering step. This might arise from testing on a moving target of a codebase. However, I got things to pick up correctly by setting make_temp_dir=False in the cleanup step.

[thomaswilliamsastro]

@low-sky I did check this run case explicitly, and got the expected behaviour, so I think this might be changes not sticking your end. Since it'll rm the temp_dir afterwards, the fact the files have moved should get picked up by the tempdir_exists() function and fall back to the main imaging directory. Can you make sure you've pulled the latest version and run again?

Otherwise, this is some real quirk of the logic I hadn't accounted for (but I've also not been running on a cluster, so you never know)

[e-koch]

@thomaswilliamsastro would you mind adding a file to the testing directory with the different testing calls for ImagingChunkedHandler? It doesn't have to run as a test right now but I think we're all using slightly different test cases and should check (a) we're using the same calls and (b) whether there are other test cases to incorporate.

[thomaswilliamsastro]

@e-koch just pushed the 6 cases I tested

[thomaswilliamsastro]

Oh, I think I see the issue. If you're farming out chunks then you'll be reinstantiating the handler at the end, which will create a temp directory and then fail...I've just pushed a patch that should fix that but it'd be good if you could check

[e-koch]

Two comments:

1. should we slightly adjust what the temp_path means to be the parent directory for the temp folder to be created in (so we always create a temp folder when requested).
2. I think we should keep the init light-weight and move copying the MS to a helper function (or at least out of init).

[e-koch]

excellent; thanks for the testing script Tom!

[e-koch]

LGTM. Anything else to address before merging @thomaswilliamsastro ?

[thomaswilliamsastro]

I've just checked the "cluster imitator" setup works fine on my end, and squashed commits, so let's merge!