-
Notifications
You must be signed in to change notification settings - Fork 165
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
Modified CDIPS output to present flux values and BTJD time #1335
base: main
Are you sure you want to change the base?
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.
You need to make sure that the flux/flux errors have the "correct" astropy unit from the HLSP
src/lightkurve/io/cdips.py
Outdated
@@ -63,6 +69,11 @@ def read_cdips_lightcurve(filename, | |||
quality_column=quality_column, | |||
time_format='btjd') | |||
|
|||
#The time displayed is in BJD not BTJD. We can fix this by subtracting | |||
# 2457000 | |||
lc["time"] = lc["time"]-2457000 |
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.
This should be taken care of with the time_format
argument by setting it to jd
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.
This has now been adjusted. Thank you.
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.
Note however that the time is given in BJD, but this does not seem to be an option.
src/lightkurve/io/cdips.py
Outdated
lc.meta["AUTHOR"] = "CDIPS" | ||
lc.meta['TARGETID'] = lc.meta.get('TICID') | ||
lc.meta['QUALITY_BITMASK'] = 36 |
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.
The bitmask needs to be updated
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.
Is the point here to change the quality bitmask to the TESS default i.e., 175?
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.
Additionally CDIPS light curves have already had quality filtering applied, and do not provide the bitflags necessary for a user to apply a new bitmask.
I recently ran into this same problem (BTJD discrepancy) - is there any chance this PR would make it in a release soon? |
@rebekah9969 was the bit mask discussion resolved? If so, is this ready to merge? |
To provide some additional context, we are soon working on adding interactive previews back into Portal using a package Kyle is writing that uses lightkurve to load the data. While not an explicit blocker, any residual PRs can be merged into a version of lightkurve would add to the number of supported datasets at initial release. If things go well on our end, that could be as soon as July 30th as part of our MAST monthly patch for July. |
PR in response to issue #1067
I have modified the cdips.py code to show the correct BTJD. I have also made IFL1 the default flux and IFE1 the default flux error.
Note that a user can still change this value back to mag or anyone of the other flux values, they just need to specify that column when downloading.
Showing the flux_error is now set as a default.
There is still an issue about what to put for the quality values. As is this is set to G for good values and X for bad. The data is filtered to only show the good data, so G is always only what is shown. Would it be better to set this to 0, this would make it more consistent with other light curve data.