Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

support mtl 1.0.0 format #742

Merged
merged 1 commit into from May 28, 2021
Merged

support mtl 1.0.0 format #742

merged 1 commit into from May 28, 2021

Conversation

sbailey
Copy link
Contributor

@sbailey sbailey commented May 28, 2021

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).

@sbailey sbailey requested a review from geordie666 May 28, 2021 06:20
@coveralls
Copy link

Coverage Status

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

@geordie666
Copy link
Contributor

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

@sbailey
Copy link
Contributor Author

sbailey commented May 28, 2021

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

@sbailey sbailey merged commit 788b7f0 into master May 28, 2021
@sbailey sbailey deleted the mtl-compatibility branch May 28, 2021 16:52
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.

None yet

3 participants