search

desihub / desitarget

DESI Targeting
BSD 3-Clause "New" or "Revised" License
18 stars 23 forks source link

support mtl 1.0.0 format #742

Closed sbailey closed 3 years ago

sbailey commented 3 years ago

This PR adds read_mtl_ledger support for the mtl/1.0.0 format, which doesn't have Z_QN but does columns ZS and ZINFO that are now gone.

Motivation: fiberassign needs latest desitarget to get the write_targets(..., subpriority=False) functionality. But, current desitarget doesn't support reading the mtl/1.0.0 format, which breaks the ability to test current fiberassign on mtl/1.0.0 files for comparison with fiberassign/4.0.0 on previously designed tiles. Thus this PR.

The solution here is admittedly fragile if the mtl format continues to evolve. I certainly hope it doesn't, but if it does change again and this becomes problematic, I suggest tackling a deeper refactor about the optimizations for speed-reading the ecsv file without requiring a specific single set of expected columns (via mtldatamodel).

coveralls commented 3 years ago

Coverage Status

Coverage decreased (-0.009%) to 58.91% when pulling 4fee94632f7b08e65e7f2d35395ea2968af9f11a on mtl-compatibility into 8e6894c7270c4f19fb8bac2c0b8921a0bd4d4950 on master.

geordie666 commented 3 years ago

This looks fragile, but it's fine to merge it if you need it to make progress.

sbailey commented 3 years ago

I will update changes.rst after merging to not trigger and wait for tests.