Skip to content

Avoid circular import between flexure and specobj,specobjs#2084

Merged
kbwestfall merged 18 commits intopypeit:developfrom
kbwestfall:spec_extract
Mar 16, 2026
Merged

Avoid circular import between flexure and specobj,specobjs#2084
kbwestfall merged 18 commits intopypeit:developfrom
kbwestfall:spec_extract

Conversation

@kbwestfall
Copy link
Collaborator

This PR does a few things to avoid circular imports among flexure, specobj, and specobjs and to modularize the code a little more:

  1. I pulled the MultiSlitFlexure class in pypeit/core/flexure.py into its own module, pypeit/multislit_flexure.py. This avoids the need to import specobjs in pypeit/core/flexure.py.
  2. I pulled out functions in pypeit/core/extract.py related to modeling the spatial profile of the object into a separate module, pypeit/core/spatialprofile.py.
  3. I removed the use of specobj in extract.extract_boxcar and extract.extract_optimal. This makes the return signatures more complicated, but it means that you don't need to construct a SpecObj object to use the low-level extraction functions.
  4. The above allowed me to refactor flexure.get_sky_spectrum so that it didn't use SpecObj.

I performed detailed tests along the way to make sure that the extracted spectra before and after the changes to the extraction algorithms yielded identical results.

Copy link
Collaborator

@profxj profxj left a comment

Choose a reason for hiding this comment

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

Definitely a bit "dirtier" in places, but now the base extraction codes are much more Pythonic.

I didn't look carefully at the code that shuffled between modules (or into new ones). Am assuming those lines didn't change much.

Copy link
Collaborator Author

@kbwestfall kbwestfall left a comment

Choose a reason for hiding this comment

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

Thanks for the review!

@kbwestfall
Copy link
Collaborator Author

Lots of test failures, but they look they were all because of 1 of 2 issues. These are now fixed, and I have restarted the tests.

@kbwestfall
Copy link
Collaborator Author

Still some lingering test failures:

Test Summary
--------------------------------------------------------
--- PYTEST PYPEIT UNIT TESTS PASSED  316 passed, 438 warnings in 256.89s (0:04:16) ---
--- PYTEST UNIT TESTS PASSED  158 passed, 1450 warnings in 767.16s (0:12:47) ---
--- PYTEST VET TESTS FAILED  1 failed, 72 passed, 1671 warnings in 5311.25s (1:28:31) ---
--- PYPEIT DEVELOPMENT SUITE FAILED 1/291 TESTS  ---
Failed tests:
    bok_bc/old_832 pypeit
Skipped tests:
Total disk usage: 316.272 GiB
Testing Started at 2026-03-05T00:05:31.389157
Testing Completed at 2026-03-05T17:38:56.088661
Total Time: 17:33:24.699504

I cannot reproduce the bok_bc/old_832 pypeit failure locally on python 3.12 (what the cloud uses) or 3.13 (what I use locally). The vet test failure is because of a correction that needed to be made to the MultiSlitFlexure import. See pypeit/PypeIt-development-suite#392 .

Copy link
Collaborator

@debora-pe debora-pe left a comment

Choose a reason for hiding this comment

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

Thanks @kbwestfall for all this hard work. This looks good.

I just had one general thought. It feels to me that explicitly filling out sobj.OPT_WAVE, sobj.OPT_COUNTS, sobj.OPT_COUNTS_IVAR, etc...every time that extract_optimal and extract_boxcar are used is (besides clunky) going to be difficult to maintain. I was wondering if we could create a function that update sobj with the outputs from extract_optimal and extract_boxcar ?
Not sure if that could be in the SpecObj class or in a separate file with all SpecObj related "operations", but in this way we would avoid to have to change all the methods that use extract_optimal and extract_boxcar whenever we make a change to their code.

@kbwestfall
Copy link
Collaborator Author

Dev-suite is still failing on the bok_bc data:

Test Summary
--------------------------------------------------------
--- PYTEST PYPEIT UNIT TESTS PASSED  330 passed, 450 warnings in 474.50s (0:07:54) ---
--- PYTEST UNIT TESTS PASSED  158 passed, 1450 warnings in 1381.33s (0:23:01) ---
--- PYTEST VET TESTS FAILED  1 failed, 73 passed, 1663 warnings in 7738.02s (2:08:58) ---
--- PYPEIT DEVELOPMENT SUITE FAILED 1/292 TESTS  ---
Failed tests:
    bok_bc/old_832 pypeit
Skipped tests:
Total disk usage: 225.097 GiB
Testing Started at 2026-03-05T22:59:33.554561
Testing Completed at 2026-03-06T20:45:02.843982
Total Time: 21:45:29.289421

The vet test failure is because it can't find the coadd2d_files/gemini_gmos_gs_ham_b600_mos.coadd2d. It was as added in a recent PR so I'm assuming that's because of some weird checkout/clone timing during the tests.

I'll try to investigate the bok_bc error a bit more...

@debora-pe
Copy link
Collaborator

Hi @kbwestfall . I run the Dev-Suite on my machine.
I noticed that the for the vet test failure, it seems there was a little bug in the test that did not show up before. Changing this line with coadd2dFile.file_paths = [Path(sci_dir).absolute()] should fix it.

The other funny thing it's that I did not get a failure for bok_bc but I got a failure for subaru_focas/300R_O58 with the error below. For reference, I checked out your PR (also the Dev Suite one) and I used Python 3.13.5.

�[1;38;2;215;48;39m   [ERROR]�[0m - �[38;2;116;173;209mspecobjs.py:get_std:345�[0m - TypeError: loop of ufunc does not support argument 0 of type numpy.ndarray which has no callable sqrt method
AttributeError: 'numpy.ndarray' object has no attribute 'sqrt'. Did you mean: 'sort'?

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/Users/dpelliccia/miniconda3/envs/pypeit/bin/run_pypeit", line 6, in <module>
    sys.exit(RunPypeIt.entry_point())
             ~~~~~~~~~~~~~~~~~~~~~^^
  File "/Users/dpelliccia/PypeIt/PypeIt/pypeit/scripts/scriptbase.py", line 118, in entry_point
    cls.main(cls.parse_args())
    ~~~~~~~~^^^^^^^^^^^^^^^^^^
  File "/Users/dpelliccia/PypeIt/PypeIt/pypeit/scripts/run_pypeit.py", line 96, in main
    pypeIt.reduce_all()
    ~~~~~~~~~~~~~~~~~^^
  File "/Users/dpelliccia/PypeIt/PypeIt/pypeit/pypeit.py", line 227, in reduce_all
    reduce_calibID(self.spectrograph, self.par, self.fitstbl,
    ~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
                                calib_ID, self.calibrations_path,
                                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
                                reduce_standard=False, overwrite=self.overwrite,
                                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
                                show=self.show, run_state=self.run_state,
                                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
                                reuse_calibs=self.reuse_calibs)
                                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/dpelliccia/PypeIt/PypeIt/pypeit/pypeit.py", line 360, in reduce_calibID
    this_spec2d, this_sobjs = exposure.reduce_exposure(
                              ~~~~~~~~~~~~~~~~~~~~~~~~^
        spectrograph, fitstbl, par, frames, calib_ID,
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    ...<2 lines>...
        show=show,
        ^^^^^^^^^^
        std_outfile=std_outfile)
        ^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/dpelliccia/PypeIt/PypeIt/pypeit/exposure.py", line 445, in reduce_exposure
    findobj_on_exposure(sciImg_dict, bkg_redux_sciimg_dict,
    ~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
                        spectrograph,
                        ^^^^^^^^^^^^^
    ...<5 lines>...
                        find_negative=find_negative,
                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
                        show=show)
                        ^^^^^^^^^^
  File "/Users/dpelliccia/PypeIt/PypeIt/pypeit/exposure.py", line 208, in findobj_on_exposure
    pypeit_steps.findobj_on_det(
    ~~~~~~~~~~~~~~~~~~~~~~~~~~~^
        sciImg, spectrograph, fitstbl, par, frames, calib_ID, det,
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
        calibrations_path,
        ^^^^^^^^^^^^^^^^^^
        bkg_redux=bkg_redux, find_negative=find_negative, show=show,
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
        std_outfile=std_outfile)
        ^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/dpelliccia/PypeIt/PypeIt/pypeit/pypeit_steps.py", line 377, in findobj_on_det
    std_trace = specobjs.get_std_trace(spectrograph.get_det_name(det),
                                    std_outfile)
  File "/Users/dpelliccia/PypeIt/PypeIt/pypeit/specobjs.py", line 1144, in get_std_trace
    sobjs_std = sobjs_det.get_std()
  File "/Users/dpelliccia/PypeIt/PypeIt/pypeit/specobjs.py", line 345, in get_std
    SNR = np.median(self.OPT_COUNTS * np.sqrt(self.OPT_COUNTS_IVAR), axis=1)
                                      ~~~~~~~^^^^^^^^^^^^^^^^^^^^^^
TypeError: loop of ufunc does not support argument 0 of type numpy.ndarray which has no callable sqrt method

@kbwestfall
Copy link
Collaborator Author

Everything passed (again) except for the coadd vet test:

Test Summary
--------------------------------------------------------
--- PYTEST PYPEIT UNIT TESTS PASSED  330 passed, 448 warnings in 378.35s (0:06:18) ---
--- PYTEST UNIT TESTS PASSED  157 passed, 1449 warnings in 1001.52s (0:16:41) ---
--- PYTEST VET TESTS FAILED  1 failed, 73 passed, 1672 warnings in 6371.14s (1:46:11) ---
--- PYPEIT DEVELOPMENT SUITE PASSED 292/292 TESTS  ---
Total disk usage: 325.354 GiB
Testing Started at 2026-03-12T15:42:01.288854
Testing Completed at 2026-03-13T10:17:09.750089
Total Time: 18:35:08.461235

I think it may be because of a difference between the directory structure in the cloud and a local install. In pypeit/PypeIt-development-suite#392, I changed all the uses of resolve to absolute, and I changed the path construction for the coadd2d file to mimic how these paths are set in the unit tests. Starting the tests again to see if this fixes it (probably overkill to run the whole thing again, but ...).

@debora-pe
Copy link
Collaborator

I think it may be because of a difference between the directory structure in the cloud and a local install. In pypeit/PypeIt-development-suite#392, I changed all the uses of resolve to absolute, and I changed the path construction for the coadd2d file to mimic how these paths are set in the unit tests. Starting the tests again to see if this fixes it (probably overkill to run the whole thing again, but ...).

Thanks @kbwestfall!
I just want to mention that I did run the tests on my machine and they pass, including the coadd2d vet test. Hopefully, the changes that you made will be the last ones!

@kbwestfall
Copy link
Collaborator Author

Tests pass!

Test Summary
--------------------------------------------------------
--- PYTEST PYPEIT UNIT TESTS PASSED  330 passed, 450 warnings in 287.40s (0:04:47) ---
--- PYTEST UNIT TESTS PASSED  157 passed, 1449 warnings in 814.73s (0:13:34) ---
--- PYTEST VET TESTS PASSED  74 passed, 1671 warnings in 5518.69s (1:31:58) ---
--- PYPEIT DEVELOPMENT SUITE PASSED 292/292 TESTS  ---
Total disk usage: 318.598 GiB
Testing Started at 2026-03-13T16:20:26.846076
Testing Completed at 2026-03-14T07:11:35.637182
Total Time: 14:51:08.791106

Copy link
Collaborator

@debora-pe debora-pe left a comment

Choose a reason for hiding this comment

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

Thanks @kbwestfall!

@kbwestfall kbwestfall merged commit 6264291 into pypeit:develop Mar 16, 2026
34 of 46 checks passed
@kbwestfall kbwestfall deleted the spec_extract branch March 16, 2026 21:32
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.

3 participants