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

Don't pass the Data Release when constructing MTL filenames #688

Merged
merged 2 commits into from Mar 20, 2021

Conversation

geordie666
Copy link
Contributor

As part of #684, we changed the data model such that the directory path to MTL files no longer includes the Data Release. This broke the MTL-specific read routines, such as desitarget.io.read_targets_in_tiles(..., mtl=True) which were still expecting to need to pass around Data Release names.

This is a very minor PR to fix that bug.

@geordie666
Copy link
Contributor Author

@araichoor: Thanks for finding this bug when reading MTL ledgers and reporting it to me. I think this PR will fix your issues. If this does work, then please feel free to merge this PR (when tests pass) if it's a blocking factor for your work.

Alternatively, I can merge this PR myself when I get out of meetings in an hour or two.

Also: Let me know if you need me to make a new tag of desitarget in the short-term so that you can make "official" use of this update.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.005%) to 59.633% when pulling 3e7459c on ADM-mtl-bup-read into 594191d on master.

@araichoor
Copy link

I confirm it runs fine now; I let you do the merging.
about a new tag: right, that may be a cleaner way to proceed (but won t change the files); if that s not too much hassle for you, yes.

@geordie666 geordie666 merged commit f88bd97 into master Mar 20, 2021
@geordie666 geordie666 deleted the ADM-mtl-bup-read branch March 20, 2021 01:48
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