Lyaretarget #714
Lyaretarget #714
Conversation
|
Thanks for pushing this. I'll try it out and make comments (although I won't hijack the branch and work on it myself until you're ready to merge). |
There was a problem hiding this comment.
I've added some initial comments just scanning through the code. I may have more comments when I've actually tried to run the code, though.
In general, this is fantastic work. You've hewed closely to the style and formalism used elsewhere in desitarget, which I really appreciate.
…s loaded to optimize.
There was a problem hiding this comment.
I tried running the code myself, and it generally works well. I appreciate that we're likely waiting for desihub/QuasarNP#4 to be resolved before QN will be fully integrated. So, in the meantime, I've added a few more comments for you to address in addition to the two outstanding comments.
…nt variables. Cleaned up argparse arguments and doc strings.
|
MgII Absorption and comparisons between QuasarNP, SQUEzE, MgII absorption, and redrock have yet to be added. I'll start on MgII absorption next. |
|
@eleanorlyke: Can we merge this to make continued progress and you can continue development in another branch? |
I haven't had a chance to add the MgII absorption yet, but if you like to merge this branch and I can make a new one for adding that functionality. |
|
Sounds good, I think I'll merge it as it's already a significant amount of new work. |
I updated the lyazcat script to incorporate QuasarNP, SQUEzE, and MgII Absorption with user options to turn off each of these as necessary (though MgII Absorption hasn't been completed yet).