christinahedges / TESS-SIP

Long term rotation period finding tool for NASA's TESS mission
MIT License
17 stars 5 forks source link

Light Curve File Collection Functionality #5

Closed tfetherolf closed 3 years ago

tfetherolf commented 3 years ago

Most of the changes are related to a new ability to use a collection of light curve files as an alternative to inputting a collection of target pixel files. The results are comparable to using the target pixel files, but the target pixel file input is still preferred. Other modifications include an added input for specific periods to search in the periodogram and the model used to correct the raw lightcurve is available as an output.

Light Curve Files: TIC288735205_SIP_lcf

Target Pixel Files: TIC288735205_SIP_tpf

christinahedges commented 3 years ago

Hi @tfetherolf! Is this PR still being worked on?

tfetherolf commented 3 years ago

@christinahedges So sorry, this fell off my radar somehow! I will get back to this over the next week or two. In the meantime, I updated the title with [WIP] as suggested.

icweaver commented 3 years ago

Hi @tfetherolf, thanks so much for this PR! Being able to use light curve files directly and the additional methods that they support has been a huge help with my current project =]

tfetherolf commented 3 years ago

Sorry it took so long to get to your feedback, @christinahedges!! I'd appreciate some help integrating basic tests. The hardest part of this for me is figuring out how GitHub works...

christinahedges commented 3 years ago

@tfetherolf, thanks for this PR! I think this is quite close to something workable, but there are a few things I'd like to change (e.g. one keyword name, some of the structure) and I'd like add tests. Rather than going back and forth on this, I'm going to merge this PR, and then add in tests in a second PR.

Thanks for all your hard work! If you want to see how the tests get added in, check out #6!

christinahedges commented 3 years ago

@tfetherolf Just to add quickly, the functionality you added does calculate a background periodogram, but that isn't valid in this case. The sap_bkg can't be used to calculate a background periodogram in the same was as pixels in the background of target pixel files, so I'm taking that functionality out.

icweaver commented 3 years ago

Thanks all for your work on this! @christinahedges regarding the background periodogram functionality you mentioned, is there another approach that we should follow, similarly to the discussion in #7?