Skip to content

Begin removal of analysisUtils - #402

Open
thomaswilliamsastro wants to merge 2 commits into
masterfrom
au-removal
Open

thomaswilliamsastro wants to merge 2 commits into
masterfrom
au-removal

Conversation

@thomaswilliamsastro

Copy link
Copy Markdown
Collaborator

This PR starts the long-overdue removal of the analysisUtils dependency. Keeping this up-to-date is difficult, complicates tests, and makes keeping the pipeline transferable annoying. This first step removes the analysisUtils dependencies for everything except the SD pipeline.

The reason the SD pipeline has been left out is because there are a lot of functions here that would need to be translated, both across the current version and the legacy pipeline. There's a decision to be made here about whether now is the time to deprecate the legacy pipeline; I think it is, but happy to hear opinions.

In any case, the shared functions have been moved into utilsSingleDish, so it'll be super easy to rip it out in the future.

Major change summary:

  • Removed the calls to au.createCasaTool(). Since we now use a unified CASA version, this is no longer needed.
  • Add a new custom estimate_synthesised_beam function. This has modifications to be better for large datasets by optionally using the first field in the dataset, and will loop over each observation ID to find a minimum beam size from each individual, concatenated observation. It's about twice as fast, and is quieter.
  • Add a new custom pick_cell_size function. This is a little different from the au equivalent. First, it'll always return the image size, and automatically pads for an FFT (we were actually doing this twice, in effect, so now the image sizes should be smaller). Second, it does not take in information about the target or the SPW, since we only ever call this at a point where we've already split out the dataset to a single target and window.
  • Add get_spw_info function, which is essentially a direct translation of the analysisUtils function, with a bit of tidying up
  • Add get_scans_for_spw function, which is essentially a direct translation of the analysisUtils function, with a bit of tidying up
  • Add get_science_spws function, which is essentially a direct translation of the analysisUtils function, with a bit of tidying up
  • Add get_chan_widths function, which is essentially a direct translation of the analysisUtils function, with a bit of tidying up
  • Add clear_pointing_table function, which is essentially a direct translation of the analysisUtils function, with a bit of tidying up
  • A couple of functions, get_sourcename and read_source_coordinates, have been moved from to casaLegacySingleDishRoutines to utilsSingleDish

Detailed changes:

  • Remove au.createCasaTool calls from casaCubeRoutines
  • Remove au.createCasaTool calls from casaFeatherRoutines
  • Remove au.createCasaTool calls from casaImagingRoutines
  • Replaced au.pickCellSize with pick_cell_and_im_size in casaImagingRoutines
  • Replaced au.ValueMapping call with get_spw_info in casaImagingRoutines
  • Remove au.createCasaTool calls from casaLegacySingleDishRoutines
  • Move get_sourcename and read_source_coordinates from casaLegacySingleDishRoutines to utilsSingleDish
  • Remove au.createCasaTool calls from casaMaskingRoutines
  • Remove au.createCasaTool calls from casaMosaicRoutines
  • Remove unused analysisUtils import from casaRoutineTests
  • Update imports for get_sourcename and read_source_coordinates in casaSingleDishALMAWrapper
  • Remove au.createCasaTool calls from casaVisRoutines
  • Add estimate_synthesised_beam function to casaVisRoutines
  • Add pick_cell_and_im_size function to casaVisRoutines
  • Add get_representative_freq function to casaVisRoutines
  • Add get_obs_ids function to casaVisRoutines
  • Add get_chan_widths function to casaVisRoutines
  • Add get_spw_info function to casaVisRoutines
  • Add get_scans_for_spw function to casaVisRoutines
  • Add get_science_spws function to casaVisRoutines
  • Add get_observatory_name function to casaVisRoutines
  • Add clear_pointing_table function to casaVisRoutines
  • Replace estimateSynthesizedBeam in clean_call
  • Remove unused analysisUtils import from handlerImaging
  • Remove au.createCasaTool calls from handlerImagingChunked
  • Remove au.createCasaTool calls from handlerTestImaging
  • Remove au.createCasaTool calls from utilsFieldSelection
  • Replaced getScienceSpws in utilsFieldSelection
  • Remove au.createCasaTool calls from utilsSingleDish
  • Remove au.createCasaTool calls from utilsTestImagingPlots
  • Update CHANGELOG.md

@thomaswilliamsastro

Copy link
Copy Markdown
Collaborator Author

@e-koch I'm sorry about this

@codecov

codecov Bot commented Sep 24, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 5.37634% with 352 lines in your changes missing coverage. Please review.
✅ Project coverage is 7.89%. Comparing base (24dcf46) to head (cae9000).
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff            @@
##           master    #402      +/-   ##
=========================================
- Coverage    7.95%   7.89%   -0.07%     
=========================================
  Files          42      42              
  Lines       16635   16814     +179     
  Branches     3983    4001      +18     
=========================================
+ Hits         1324    1327       +3     
- Misses      15288   15464     +176     
  Partials       23      23              
Components Coverage Δ
casa 4.83% <4.04%> (-0.12%) ⬇️
handler 6.25% <0.00%> (-0.05%) ⬇️
spectral-cube 10.14% <ø> (ø)
utils 13.58% <13.04%> (-0.01%) ⬇️

@e-koch e-koch left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, Tom! This is going to simplify our testing and version support a lot.

Added a few comments on tracking which new functions are primarily from analysisUtils so we recall. And if we haven't already, we should include the analysisUtils license where relevant. (We could move all of the adopted analysisUtils functions into their own file; not as clean for the purpose of the functions but easier for credit and provenance).

Do we need any additional testing for consistency? Have you already handled that with local tests?

return obs_ids


def get_chan_widths(

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@thomaswilliamsastro -- how much of the analysisUtils code do we copy over (in general)? We may have a place elsewhere but we should be sure to include the analysisUtils license where we've taken appreciable amounts of code.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and more practically: I suggest we note inthe docstring which functions are largely adapted from analysisUtils so we recall in the future

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair point, added this info in, point at the Zenodo DOI, and the license information for the functions that are mostly taken wholesale

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are these our tests of analysisUtils? Just for imports or could they be useful for consistency checks running on a test MS?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For now they just test the functions exist and any keywords in the pipeline we pass to them also exist. We don't do any consistency checks on the actual results of the functions (yet), so this is just removing au functions that no longer exist

This PR starts the long-overdue removal of the analysisUtils dependency. Keeping this up-to-date is difficult, complicates tests, and makes keeping the pipeline transferable annoying. This first step removes the analysisUtils dependencies for everything except the SD pipeline.

The reason the SD pipeline has been left out is because there are *a lot* of functions here that would need to be translated, both across the current version and the legacy pipeline. There's a decision to be made here about whether now is the time to deprecate the legacy pipeline; I think it is, but happy to hear opinions.

In any case, the shared functions have been moved into utilsSingleDish, so it'll be super easy to rip it out in the future.

Major change summary:

- Removed the calls to au.createCasaTool(). Since we now use a unified CASA version, this is no longer needed.
- Add a new custom estimate_synthesised_beam function. This has modifications to be better for large datasets by optionally using the first field in the dataset, and will loop over each observation ID to find a minimum beam size from each individual, concatenated observation. It's about twice as fast, and is quieter.
- Add a new custom pick_cell_size function. This is a little different from the au equivalent. First, it'll always return the image size, and automatically pads for an FFT (we were actually doing this twice, in effect, so now the image sizes should be smaller). Second, it does not take in information about the target or the SPW, since we only ever call this at a point where we've already split out the dataset to a single target and window.
- Add get_spw_info function, which is essentially a direct translation of the analysisUtils function, with a bit of tidying up
- Add get_scans_for_spw function, which is essentially a direct translation of the analysisUtils function, with a bit of tidying up
- Add get_science_spws function, which is essentially a direct translation of the analysisUtils function, with a bit of tidying up
- Add get_chan_widths function, which is essentially a direct translation of the analysisUtils function, with a bit of tidying up
- Add clear_pointing_table function, which is essentially a direct translation of the analysisUtils function, with a bit of tidying up
- A couple of functions, get_sourcename and read_source_coordinates, have been moved from to casaLegacySingleDishRoutines to utilsSingleDish
- Updated tests to remove those no longer needed

Detailed changes:

- Remove au.createCasaTool calls from casaCubeRoutines
- Remove au.createCasaTool calls from casaFeatherRoutines
- Remove au.createCasaTool calls from casaImagingRoutines
- Replaced au.pickCellSize with pick_cell_and_im_size in casaImagingRoutines
- Replaced au.ValueMapping call with get_spw_info in casaImagingRoutines
- Remove au.createCasaTool calls from casaLegacySingleDishRoutines
- Move get_sourcename and read_source_coordinates from casaLegacySingleDishRoutines to utilsSingleDish
- Remove au.createCasaTool calls from casaMaskingRoutines
- Remove au.createCasaTool calls from casaMosaicRoutines
- Remove unused analysisUtils import from casaRoutineTests
- Update imports for get_sourcename and read_source_coordinates in casaSingleDishALMAWrapper
- Remove au.createCasaTool calls from casaVisRoutines
- Add estimate_synthesised_beam function to casaVisRoutines
- Add pick_cell_and_im_size function to casaVisRoutines
- Add get_representative_freq function to casaVisRoutines
- Add get_obs_ids function to casaVisRoutines
- Add get_chan_widths function to casaVisRoutines
- Add get_spw_info function to casaVisRoutines
- Add get_scans_for_spw function to casaVisRoutines
- Add get_science_spws function to casaVisRoutines
- Add get_observatory_name function to casaVisRoutines
- Add clear_pointing_table function to casaVisRoutines
- Replace estimateSynthesizedBeam in clean_call
- Remove unused analysisUtils import from handlerImaging
- Remove au.createCasaTool calls from handlerImagingChunked
- Remove au.createCasaTool calls from handlerTestImaging
- Remove au.createCasaTool calls from utilsFieldSelection
- Replaced getScienceSpws in utilsFieldSelection
- Remove au.createCasaTool calls from utilsSingleDish
- Remove au.createCasaTool calls from utilsTestImagingPlots
- Update tests
- Update CHANGELOG.md
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants