LyA Retartgeting Changes

[eleanorlyke]

Made changes to io and utils to integrate better with LyA retargeting. Added a "load_desi_coadd()" function to use coadd files over exposures or spectrum files (io) and added a cbest output alongside the zbest output (utils) for the LyA zqso files.

[geordie666]

@sbailey: I don't think this is needed:

• update load_desi_coadd to filter by targetid using np.isin instead of filtering by fibers, which only applies to tilepetal-based coadds but not future healpix-based coadds

So, I say let's not do that. All of the understanding of which TARGETIDs to handle will be in the desitarget.mtl module. I think it's overkill to have that filtering in two places.

calling the kwarg fibers here is a little misleading, though. I say just change that to rows and call your second bullet point done. If somebody wants to filter by TARGETID, they'd just work out which rows in the file contained the needed TARGETIDs in advance and pass True for those rows.

[geordie666]

I agree with this point though:

with a default of None meaning "read everything"

I'd just change this: , or otherwise being list/array of targetids to use. to ,or otherwise being a list/array of rows to read from the file (where True means read that row and False means don't read that row).

[sbailey]

@geordie666 @eleanorlyke ok, if you want to change fibers to rows that's fine. The main point is to not hardcode that the files have 500 rows or assume that row i is fiber i. Although that happens to be true for the current coadds, that isn't true in general for coadds.

[geordie666]

@sbailey: Sounds good. I agree that the main goal here is not to encode "500" anywhere.

[dylanagreen]

With regards to abstracting reused code, I will take care of that. My plan is to basically leave the return API of load_desi_exposure unchanged (which maintains compatibility) but I will abstract out reusable code portions (like renormalizing the data) and leave anything specific to loading cframe files in that function. Then these reusable "blocks" can be then just plugged into load_desi_coadd, something which I can/will also handle that I don't think is necessary to be implemented for this specific pull request. Given that, I approve the changes left in the PR and am comfortable pulling if @sbailey and @dkirkby approve.

[geordie666]

@sbailey and @dkirkby: Can we merge this? It's a blocking factor for continued development in desitarget.

[sbailey]

Thanks for the ping. Merging now.