-
Notifications
You must be signed in to change notification settings - Fork 23
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
restore w1/w2 flux argument to iselg_colors #514
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the quick work on this. One comment:
I think you'll need the exact same changes in py/desitarget/sv1/sv1_cuts.py
? Specifically here:
https://github.com/desihub/desitarget/blob/elg-w12flux/py/desitarget/sv1/sv1_cuts.py#L903
A second comment:
You'll also need to pass w1flux
and w2flux
to isELG()
. Here:
https://github.com/desihub/desitarget/blob/elg-w12flux/py/desitarget/sv1/sv1_cuts.py#L853-L854
and in the sv1_cuts.py
code.
I think this will work, now. Strictly, for completeness, you might want to pass desitarget/py/desitarget/cuts.py Line 259 in d151c91
That's your call (I think the code will work without failing, either way). Feel free to merge at your leisure. |
Although I accidentally retriggered tests, I'm going to go ahead and merge. I tested Thanks for your help, @geordie666. |
Excellent. If you don't think the mocks are going to fail on desitarget/py/desitarget/cuts.py Lines 1938 to 1947 in 01c930a
Then we should be good to go. Apologies for breaking this at such short notice. |
Sorry, missed your last comment. If |
restore w1/w2 flux argument to iselg_colors
Near-trivial fix for an unintended change to the
isELG
API introduced in #513.