kammerje / spaceKLIP

Pipeline for reducing JWST high-contrast imaging data. Published in Kammerer et al. 2022 and Carter et al. 2022.
https://ui.adsabs.harvard.edu/abs/2022SPIE12180E..3NK/abstract
MIT License
16 stars 10 forks source link

Tutorial notebooks and some code improvements for version 2 #84

Closed mperrin closed 1 year ago

mperrin commented 1 year ago

Now also includes the contents formerly in draft PR #81 to improve database and plotting tools for compatibility with level 3 outputs.

mperrin commented 1 year ago

This branch now also includes various small fixes and enhancements in the code, based on test runs of the tutorial notebooks. See #87

mperrin commented 1 year ago

@AarynnCarter I wanted to check in with you about next steps for this branch with the tutorial notebooks. Whether it might make sense to review and merge in what we have here so far, if there are specific things we should fix or work to improve further before a potential merge, etc.

@juliengirard I think you had said that you might try running through some of the tutorials notebooks here yourself also? Were you able to have time to do so yet, out of curiosity?

I'm partially just trying to catch up and remember where we were, now that I'm back from vacation and getting back into the swing of things... I know there's a lot going on with white papers and upcoming conference etc so it's fine to let this wait a while as well. Just asking your thoughts.

AarynnCarter commented 1 year ago

@mperrin I'm wondering if we should hold off a little bit, this is all great work already but something is telling me we should get the calibrated contrast functionality merged into the main branch first. Then we can release a more dedicated documentation update that covers all aspects from image processing, raw contrasts, calibrated contrasts, and companion fitting. In the meantime, these tutorials aren't locked away too, so we can always point people directly to this branch as necessary.

That's not to say we should cover everything with this pull, but I think it would be nice to have a release where we can announce to the community that we've spent a lot of time working on the code and now we have end-to-end documentation available alongside a more flexible and user friendly code framework. This could also give us a nice opportunity to test the code. Since spaceKLIPs inception we've (I've) been pushing a lot of haphazard and/or small updates and it's probably worth being more intentional about pushing towards specific milestones.

Perhaps as a middle ground we could create a dedicated dev staging branch where we can merge multiple small and large branches before making milestone merges to the main branch? Happy to bounce some ideas around / interested what you think.

mperrin commented 1 year ago

👍 Order of operations for which PR(s) to do first was exactly the sort of direction I was hoping you could clarify to me. Got it. Can prioritize #91 ahead of this one, will do.

And yes, I would be strongly in favor of having a dev branch. My standard approach for poppy, webbpsf and many other packages has been to have branches develop and stable, to clearly separate in-development merged PRs from stable "ready for general use" code. So the pattern becomes many individual PRs -> get merged into develop over time -> eventually get ready for a release -> merge develop to stable (or main equivalently) and tag as a release.

AarynnCarter commented 1 year ago

Okay sounds good, I will make a develop branch now. In that case I think it is okay to merge this branch into develop as is, then we can make a new branch to add documentation for more advanced aspects like contrast calibration and companion fitting once the actual contrast calibration code has been merged into develop also.

mperrin commented 1 year ago

Updated PR to point to develop instead of main.

mperrin commented 1 year ago

I will make a develop branch now. In that case I think it is okay to merge this branch into develop as is.

Copy that. Doing so now.