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

Added fixes for some bugs in the to_fits methods of LightCurve classes #1345

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

christinahedges
Copy link
Collaborator

This PR fixes bugs in the LightCurve, KeplerLightCurve, and TESSLightCurve to_fits methods and the tests. This ensures users can pass extra keyword arguments.

@Nschanche points out that it's odd that, by default, to_fits doesn't save every column. Some of the columns can be a bit superfluous, so I'm not sure of a neat way forward here. What do people think, should to_fits save all columns by default?

@christinahedges christinahedges added 🐛 bug Something isn't working ☑️ feedback wanted This work asks for some feedback! 💪 WIP This is a work in progress! labels Jun 13, 2023
@christinahedges
Copy link
Collaborator Author

@Nschanche when you review this you'll see I've accidentally linted this file....but I don't know that I hate that...

@Nschanche
Copy link
Collaborator

@Nschanche points out that it's odd that, by default, to_fits doesn't save every column. Some of the columns can be a bit superfluous, so I'm not sure of a neat way forward here. What do people think, should to_fits save all columns by default?

There are a number of keywords that are dropped by to_fits as it is now, including timecorr, sap_flux, sap_bkg, sap_bkg_err, psf_centr1+2 and errors, and pos_cor1+2 (the last two are addressed in this pull request). I tend to think everything should be saved by default, but we could make it an explicit choice by, e.g., adding a flag such as all=True that would save every column, but stick with the current subset by default.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
☑️ feedback wanted This work asks for some feedback! 🐛 bug Something isn't working 💪 WIP This is a work in progress!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants