spacetelescope / lcviz

light curve visualization and analysis tool
https://lcviz.readthedocs.io/
BSD 3-Clause "New" or "Revised" License
5 stars 9 forks source link

"flatten" deletes data points #142

Open ttdu opened 1 month ago

ttdu commented 1 month ago

Bug: After using flatten, some data disappears. Expected behavior: flatten remove long-term trends but the number of data points is preserved

Steps to reproduce the bug:

  1. visit https://mast.stsci.edu/viz/ui/#/lcviz?uri=mast%3AK2%2Furl%2Fmissions%2Fk2%2Flightcurves%2Fc19%2F246400000%2F49000%2Fktwo246449092-c19_llc.fits
  2. use "flatten"
  3. All of the data from 9-14 days after start is completely obliterated
kecnry commented 1 month ago

The original points in this case have a flux of exactly zero and it appears that lightkurve's faltten results in nans for these.

from lightkurve import search_lightcurve
lc = search_lightcurve("ktwo246449092", mission="K2", campaign=19, cadence="long").download()
print(lc[-1])
print(lc.flatten()[-1])

I suspect this isn't actually a bug, but is actually doing as expected (although it is confusing to see points that used to be plotted no longer exist - so maybe we can at least detect this in advance and warn that it will happen). @christinahedges - is this right? Would you consider this the same as https://github.com/lightkurve/lightkurve/issues/674?

ttdu commented 1 month ago

One thing that is handled by lcviz is the selected time range for plotting: if fluxes are NaNs, pressing the "home" button to zoom to all visible data is misleading. The NaNs aren't visible, so they shouldn't be in the "visible" range

kecnry commented 1 month ago

It's a good point, but the times aren't nans, only the fluxes. So, in order to exclude those points from the zoom reset logic, we'd need to filter all x-values by whether their respective y-values are nans or not. I'm honestly not sure what is the more desired behavior in this case. Any thoughts @bmorris3?