Repository: lightkurve/lightkurve

Issue:
`lc.bin()` failed for normalized FoldedLightCurve with AttributeError: 'Quantity' object has no 'format' member
<!-- Fill in the information below before opening an issue. -->

#### Problem description
<!-- Provide a clear and concise description of the issue. -->

When trying to perform `bin()` for a normalized FoldedLightCurve, created from `lc.fold(...,  normalize_phase=True)`, one would get `AttributeError: 'Quantity' object has no 'format' member`.

The underlying issue is that `bin()` implementation assumes `lc.time` is `Time` / `TimeDelta`, but for normalized FoldedLightCurve, `lc.time` is `Quantity` with unit `u.dimensionless_unscaled` (conceptually floats).

#### Example
<!-- Provide a link or minimal code snippet that demonstrates the issue. -->
```python
import lightkurve as lk
# insert code here ...

lc = lk.search_lightcurve("TIC261136679", mission="TESS", cadence="short")[-1].download()  # pi Men

lc.fold(epoch_time=3011.551, period=6.26791).bin(n_bins=100).scatter();  # okay


# The following all failed with AttributeError: 'Quantity' object has no 'format' member

lc.fold(epoch_time=3011.551, period=6.26791, normalize_phase=True).bin(n_bins=100).scatter();

lc.fold(epoch_time=3011.551, period=6.26791, normalize_phase=True).bin(bins=100).scatter();

lc.fold(epoch_time=3011.551, period=6.26791, normalize_phase=True).bin(time_bin_size=0.05*u.dimensionless_unscaled).scatter();

```

Example error / stacktrace:
```python
...
File C:\pkg\_winNonPortables\miniforge3\envs\my_lk_plus\lib\site-packages\lightkurve\lightcurve.py:1519, in LightCurve.bin(self, time_bin_size, time_bin_start, time_bin_end, n_bins, aggregate_func, bins, binsize)
   1514         time_bin_start = TimeDelta(
   1515             time_bin_start, format=self.time.format, scale=self.time.scale
   1516         )
   1517     else:
   1518         time_bin_start = Time(
-> 1519             time_bin_start, format=self.time.format, scale=self.time.scale
   1520         )
   1522 # Backwards compatibility with Lightkurve v1.x
   1523 if time_bin_size is None:

File C:\pkg\_winNonPortables\miniforge3\envs\my_lk_plus\lib\site-packages\astropy\units\quantity.py:1108, in Quantity.__getattr__(self, attr)
   1103 """
   1104 Quantities are able to directly convert to other units that
   1105 have the same physical type.
   1106 """
   1107 if not self._include_easy_conversion_members:
-> 1108     raise AttributeError(
   1109         f"'{self.__class__.__name__}' object has no '{attr}' member"
   1110     )
   1112 def get_virtual_unit_attribute():
   1113     registry = get_current_unit_registry().registry

AttributeError: 'Quantity' object has no 'format' member
```
#### Expected behavior
<!-- Describe the behavior you expected and how it differs from the behavior observed in the example. -->

`bin()` should work on normalized FoldedLightCurve.

#### Environment

-  lightkurve version (e.g. 1.0b6): 2.4.2
-  installation method (e.g. pip, conda, source): conda

Discussion:
@Nschanche: Thanks for pointing this out @orionlee . We are working on some improvements for binning, so we will think about how we can address this as well. @Nschanche: Issue resolved with #1491

Solution patch:
diff --git a/CHANGES.rst b/CHANGES.rst
index 53641dce2..473033206 100644
--- a/CHANGES.rst
+++ b/CHANGES.rst
@@ -11,7 +11,10 @@
 - Updated the default TESS quality mask to include Argabrightening, Impulsive Outlier, and Bad Calibration Exclude flags (see issue #1230). [#1336]
 - Updated tutorial notebook links [#1484]
 - Author keyword now specifies between SPOC and TESS-SPOC [#1487]
-- Changed calls of `np.in1d` to `np.isin` for numpy v2 compatibility [#1492]
+- Fixed the bug in ``bin()`` function with ``bin`` parameter for ``FoldedLightCurve`` objects. [#1491]
+- Modified ``copy()`` and ``bin()`` functions for ``FoldedLightCurve`` objects to fix issues with normalized phase [#1491]
+- Removed ``binsize`` and ``time_bin_end`` as input options for ``FoldedLightCurve`` objects [#1491]
+- Changed calls of ```np.in1d``` to ``np.isin`` for numpy v2 compatibility [#1492]
 
 2.5.0 (2024-08-29)
 =====================
diff --git a/src/lightkurve/lightcurve.py b/src/lightkurve/lightcurve.py
index 0638acbcf..432174aa3 100644
--- a/src/lightkurve/lightcurve.py
+++ b/src/lightkurve/lightcurve.py
@@ -65,6 +65,7 @@ def _is_np_structured_array(data1):
     return isinstance(data1, np.ndarray) and data1.dtype.names is not None
 
 
+
 class LightCurve(TimeSeries):
     """
     Subclass of AstroPy `~astropy.table.Table` guaranteed to have *time*, *flux*, and *flux_err* columns.
@@ -1036,11 +1037,17 @@ def fold(
         if (
             epoch_phase is not None
             and not isinstance(epoch_phase, Quantity)
-            and not normalize_phase
         ):
-            epoch_phase *= u.day
+            if not normalize_phase:
+                epoch_phase *= u.day
+            else:
+                epoch_phase *= u.dimensionless_unscaled
         if wrap_phase is not None and not isinstance(wrap_phase, Quantity):
-            wrap_phase *= u.day
+            if normalize_phase:
+                wrap_phase *= u.dimensionless_unscaled
+            else:
+                wrap_phase *= u.day
+                
 
         # Warn if `epoch_time` appears to use the wrong format
         if epoch_time is not None and epoch_time.value > 2450000:
@@ -1071,17 +1078,21 @@ def fold(
         # `normalize_phase=True`, so creating a `FoldedLightCurve` object
         # requires the following three-step workaround:
         # 1. Give the folded light curve a valid time column again
+
         with ts._delay_required_column_checks():
             folded_time = ts.time.copy()
             ts.remove_column("time")
             ts.add_column(self.time, name="time", index=0)
+
         # 2. Create the folded object
         lc = FoldedLightCurve(data=ts)
+
         # 3. Restore the folded time
         with lc._delay_required_column_checks():
             lc.remove_column("time")
             lc.add_column(folded_time, name="time", index=0)
 
+
         # Add extra column and meta data specific to FoldedLightCurve
         lc.add_column(
             self.time.copy(), name="time_original", index=len(self._required_columns)
@@ -1582,6 +1593,7 @@ def bin(
                 time_bin_size=time_bin_size,
                 n_bins=n_bins,
                 time_bin_start=time_bin_start,
+                time_bin_end=time_bin_end,
                 aggregate_func=aggregate_func,
                 **kwargs
             )
@@ -1598,6 +1610,7 @@ def bin(
                     time_bin_size=time_bin_size,
                     n_bins=n_bins,
                     time_bin_start=time_bin_start,
+                    time_bin_end=time_bin_end,
                     aggregate_func=rmse_func,
                 )
                 ts["flux_err"] = ts_err["flux_err"]
@@ -1608,6 +1621,7 @@ def bin(
                     time_bin_size=time_bin_size,
                     n_bins=n_bins,
                     time_bin_start=time_bin_start,
+                    time_bin_end=time_bin_end,
                     aggregate_func=np.nanstd,
                 )
                 ts["flux_err"] = ts_err["flux"]
@@ -1838,6 +1852,7 @@ def _create_plot(
         self,
         method="plot",
         column="flux",
+        time_column='time',
         ax=None,
         normalize=False,
         xlabel=None,
@@ -1858,6 +1873,8 @@ def _create_plot(
             One of 'plot', 'scatter', or 'errorbar'.
         column : str
             Name of data column to plot. Default `flux`.
+        time_column : str
+            Name of time data column. Defauld `time`. 
         ax : `~matplotlib.axes.Axes`
             A matplotlib axes object to plot into. If no axes is provided,
             a new one will be generated.
@@ -1892,18 +1909,20 @@ def _create_plot(
         ax : `~matplotlib.axes.Axes`
             The matplotlib axes object.
         """
+        flux = self[column]
+        time = self[time_column]
         # Configure the default style
         if style is None or style == "lightkurve":
             style = MPLSTYLE
         # Default xlabel
         if xlabel is None:
-            if not hasattr(self.time, "format"):
+            if not hasattr(time, "format"):
                 xlabel = "Phase"
-            elif self.time.format == "bkjd":
+            elif time.format == "bkjd":
                 xlabel = "Time - 2454833 [BKJD days]"
-            elif self.time.format == "btjd":
+            elif time.format == "btjd":
                 xlabel = "Time - 2457000 [BTJD days]"
-            elif self.time.format == "jd":
+            elif time.format == "jd":
                 xlabel = "Time [JD]"
             else:
                 xlabel = "Time"
@@ -1967,7 +1986,7 @@ def _create_plot(
             if ax is None:
                 fig, ax = plt.subplots(1)
             if method == "scatter":
-                sc = ax.scatter(self.time.value, flux, **kwargs)
+                sc = ax.scatter(time.value, flux, **kwargs)
                 # Colorbars should only be plotted if the user specifies, and there is
                 # a color specified that is not a string (e.g. 'C1') and is iterable.
                 if (
@@ -1983,12 +2002,13 @@ def _create_plot(
             elif method == "errorbar":
                 if np.any(~np.isnan(flux_err)):
                     ax.errorbar(
-                        x=self.time.value, y=flux.value, yerr=flux_err.value, **kwargs
+                        x=time.value, y=flux.value, yerr=flux_err.value, **kwargs
                     )
                 else:
                     log.warning(f"Column `{column}` has no associated errors.")
             else:
-                ax.plot(self.time.value, flux.value, **kwargs)
+                ax.plot(time.value, flux.value, **kwargs)
+
             # Default title (none)
             if title is not None:
                 ax.set_title(title)
@@ -3054,6 +3074,46 @@ def even_mask(self):
         See the documentation of `odd_mask` for examples.
         """
         return ~self.odd_mask
+    
+    def _replace_normalized_phase(self):
+        # Some astropy functions, such as aggregate_downsample, require Time or TimeDelta
+        # As normalized phase-folded lightcurves are unitless, this breaks
+        # This will replace the normalized phase with phase in TimeDelta
+
+        # If the lightcurve is phase folded but not normalized, just return self
+        if not self.meta.get("NORMALIZE_PHASE"):
+            # it should never happen, likely that there is some bug.
+            warnings.warn("The function should be invoked on a folded lightcurve with normalized phase. No-Op.")
+            return
+
+        # If the phase folded lightcurve is normalized, unnormalize it
+        with self._delay_required_column_checks():
+            normalized_phase = self.time
+            #print(normalized_phase * self.period)
+            phase = TimeDelta(normalized_phase * self.period)
+            self.remove_column("time")
+            self.add_column(phase, name="time", index=0)
+        #return self
+
+
+    def _restore_normalized_phase(self):
+        # Some astropy functions, such as aggregate_downsample, require Time or TimeDelta
+        # As normalized phase-folded lightcurves are unitless, this breaks
+        # This will re-normalized the phase
+
+        # Checki if the lightcurve is already normalized
+        if isinstance(self.time, Quantity):
+            # should not happen, likely there is some bug
+            warnings.warn("Attempt to restore normalized phase while the phase has already been normalized. No-op.")
+            return 
+        
+        with self._delay_required_column_checks():
+            phase = self.time
+            normalized_phase = phase / self.period
+            self.remove_column("time")
+            self.add_column(normalized_phase, name="time", index=0)
+
+            
 
     def _set_xlabel(self, kwargs):
         """Helper function for plot, scatter, and errorbar.
@@ -3063,6 +3123,10 @@ def _set_xlabel(self, kwargs):
             kwargs["xlabel"] = "Phase"
             if isinstance(self.time, TimeDelta):
                 kwargs["xlabel"] += f" [{self.time.format.upper()}]"
+            if self.normalize_phase == True:
+                kwargs["xlabel"] += f" (Normalized)"
+
+                    
         return kwargs
 
     def plot(self, **kwargs):
@@ -3140,7 +3204,141 @@ def plot_river(self, **kwargs):
             period=self.period, epoch_time=self.epoch_time, **kwargs
         )
         return ax
+    
+
+    def bin(
+        self,
+        time_bin_size=None,
+        time_bin_start=None,
+        aggregate_func=None,
+        bins=None,
+        n_bins=None,
+
+    ):
+        """Bins a FoldedLightCurve in equally-spaced bins in phase.
+        Binning always occurs in time units (not normalized phase units)
+
+        If the original light curve contains flux uncertainties (``flux_err``),
+        the binned lightcurve will report the root-mean-square error.
+        If no uncertainties are included, the binned curve will return the
+        standard deviation of the data.
 
+        Parameters
+        ----------
+        time_bin_size : `~astropy.units.Quantity`,`~astropy.time.TimeDelta`, or scalar (optional)
+            The time interval for the binned time series - this is either a scalar
+            value (in which case all time bins will be assumed to have the same
+            duration) or as an array of values (in which case each phase bin can
+            have a different duration). In cases where the lightcurve is phase-normalized, a scalar
+            input will be assumed to be in normalized phase units (u.dimensionless_unscaled) 
+            (Default: 0.5 days; default unit: days.)
+        time_bin_start : Time like value, optional
+            The start phase for the binned time series. This can also be a scalar
+            value if ``time_bin_size`` is provided. Defaults to the first
+            time in the sampled time series.
+        aggregate_func : callable, optional
+            The function to use for combining points in the same bin. Defaults
+            to np.nanmean.
+        bins : int, optional
+            int which gives the number of bins to divide the lightkurve into.
+            This adjusts the length of ``time_bin_size``
+            to accommodate the input time series length.
+        n_bins: 
+            This functionality is deprecated for FoldedLightCurves. If provided, asserts
+            bins=n_bins
+
+        Returns
+        -------
+        binned_lc : `FoldedLightCurve`
+            A new folded light curve which has been binned, with the 'time' column phase in days 
+        """
+        # astropy's aggregate_downsample function only works when 'time' is type Time or TimeDelta
+        # Since the normalized phase is a unitless quantity, this breaks.
+        # To work around this, we reset the index to be the regular phase (TimeDelta) when binning
+        
+        if n_bins != None:
+            warnings.warn("n_bins is no longer accepted for FoldedLightCurve objects. Please specify 'bins' instead")
+            bins = n_bins
+        if bins != None:
+            if not isinstance(bins, int):
+                raise ValueError('bins must be an integer describing the total number of bins.')
+            if time_bin_size != None:
+                raise ValueError("Can not specify both 'bins' and 'time_bin_size'")
+            
+
+        if self.normalize_phase == True:
+            self._replace_normalized_phase()
+
+        if time_bin_start is None:
+            time_bin_start = self.time[0]
+        if isinstance(time_bin_start, list):
+            time_bin_start = time_bin_start[0]
+            warnings.warn("FoldedLightCurve does not support lists for time_bin_start. Using the first value.")
+        if isinstance(time_bin_start, Quantity):
+            if time_bin_start.unit == u.dimensionless_unscaled:
+                    time_bin_start = time_bin_start.value * self.period      
+        if np.isscalar(time_bin_start):
+            time_bin_start = time_bin_start * u.day
+
+
+
+        if time_bin_size != None:
+            if np.isscalar(time_bin_size):
+                if self.normalize_phase == True:
+                    time_bin_size = time_bin_size * u.dimensionless_unscaled
+                else:
+                    time_bin_size = time_bin_size * u.day
+            if isinstance(time_bin_size, Quantity):
+                if time_bin_size.unit == u.dimensionless_unscaled:
+                    time_bin_size = time_bin_size.value * self.period
+ 
+
+        if bins != None:
+            # parent bin(bins=###) assumes time has format 'mjd', so does not work for phase data
+            # AttributeError: 'TimeDelta' object has no attribute 'mjd'
+            # So we directly compute the time_bin_size if the number of bins is provided
+            if time_bin_size != None:
+                raise ValueError("Can not specify both 'bins' and 'time_bin_size'")
+            if time_bin_start != self.time[0]:
+                time_bin_size = (np.nanmax(self.time) - time_bin_start).to('day') / bins
+            else:
+                time_bin_size = self.period / bins
+
+
+        result = super().bin(
+                        time_bin_size=time_bin_size,
+                        time_bin_start=time_bin_start,
+                        aggregate_func=aggregate_func,
+                    )
+        
+        if self.normalize_phase == True:
+            self._restore_normalized_phase()
+            result._restore_normalized_phase()
+        
+        return result 
+    
+
+    def copy(self):
+        # the default copy() in astropy fails if the lightcurve
+        # has normalized phase (time column is `Quantity` instead of `Time` like)
+        # workaround it by temporarily changing time column to 
+        # non-normalized `TimeDelta`
+    
+        if self.normalize_phase == True:
+            self._replace_normalized_phase()
+        
+        result = super().copy()
+
+        if self.normalize_phase == True:
+            self._restore_normalized_phase()
+            result._restore_normalized_phase()
+        return result
+    
+
+    
+
+
+    
 
 class KeplerLightCurve(LightCurve):
     """Subclass of :class:`LightCurve <lightkurve.lightcurve.LightCurve>`
@@ -3323,7 +3521,7 @@ def to_fits(
         aperture_mask=None,
         **extra_data,
     ):
-        """Writes the KeplerLightCurve to a FITS file.
+        """Writes the TessLightCurve to a FITS file.
 
         Parameters
         ----------
@@ -3394,7 +3592,7 @@ def _boolean_mask_to_bitmask(aperture_mask):
     aperture_mask : array-like
         2D aperture mask. The mask can be either a boolean mask or an integer
         mask mimicking the Kepler/TESS convention; boolean or boolean-like masks
-        are converted to the Kepler/TESS conventions.  Kepler bitmasks are
+        are converted to Kepler/TESS conventions.  Kepler bitmasks are
         returned unchanged except for possible datatype conversion.
 
     Returns


Test patch:
diff --git a/tests/test_lightcurve.py b/tests/test_lightcurve.py
index 43222b509..38630a122 100644
--- a/tests/test_lightcurve.py
+++ b/tests/test_lightcurve.py
@@ -257,6 +257,7 @@ def test_lightcurve_fold():
     assert_array_equal(np.sort(fold.time_original), lc.time)
     assert len(fold.time_original) == len(lc.time)
     fold = lc.fold(period=1, epoch_time=-0.1)
+    assert_almost_equal(fold.phase[0], -0.5, 2)
     assert_almost_equal(fold.time[0], -0.5, 2)
     assert_almost_equal(np.min(fold.phase), -0.5, 2)
     assert_almost_equal(np.max(fold.phase), 0.5, 2)
@@ -266,7 +267,7 @@ def test_lightcurve_fold():
         # `transit_midpoint` is deprecated and its use will emit a warning
         warnings.simplefilter("ignore", LightkurveWarning)
         fold = lc.fold(period=1, transit_midpoint=-0.1)
-    assert_almost_equal(fold.time[0], -0.5, 2)
+    assert_almost_equal(fold.phase[0], -0.5, 2)
     ax = fold.plot()
     assert "Phase" in ax.get_xlabel()
     ax = fold.scatter()
@@ -280,6 +281,33 @@ def test_lightcurve_fold():
     with pytest.warns(LightkurveWarning, match="appears to be given in JD"):
         lc.fold(10, 2456600)
 
+    # Make sure binning a phase-normalized folded lightcurve works (#1422)
+    fold = lc.fold(period=1.5, normalize_phase=False)
+    assert isinstance(fold.time, TimeDelta)
+    assert_almost_equal(np.max(fold.phase)-np.min(fold.phase), 1.5, 1)
+    assert len(fold.bin(bins=10)) == 10
+    fold = lc.fold(period=1.5, normalize_phase=True)
+    assert isinstance(lc.time, Time)
+    assert isinstance(fold.time, u.Quantity)
+    assert fold.time.unit == u.dimensionless_unscaled
+    assert_almost_equal(np.max(fold.phase)-np.min(fold.phase), 1, 1)
+    binned = fold.bin(bins=10)
+    assert len(binned) == 10
+    # ensure fold was not changed
+    assert len(fold) == 100
+    assert isinstance(fold.time, u.Quantity)
+
+    # Make sure 'copy()' works as expected
+    fold_copy = fold.copy()
+    assert_array_equal(fold.time, fold_copy.time)
+    assert_array_equal(fold.flux, fold_copy.flux)
+    # ensure the it is a deep copy
+    assert fold is not fold_copy
+    assert fold.time is not fold_copy.time
+    assert fold.flux is not fold_copy.flux
+
+
+
 
 @pytest.mark.parametrize(
     "normalize_phase", [False, True]
@@ -306,6 +334,12 @@ def test_lightcurve_fold_odd_even_masks(normalize_phase):
     assert len(odd) == len(fold.time)
     assert np.all(odd == ~even)
 
+    # Check wrap_phase keyword works as expected for normalized folded lightcurves (see #1423)
+    wrapped_fold = lc.fold(period=period, epoch_time=epoch_time, epoch_phase=0.5, normalize_phase=normalize_phase, wrap_phase=0.25)
+    assert_almost_equal(wrapped_fold.phase[-1].value, 0.25, decimal = 1)
+
+
+
     # cycle 0: time [0, 1)
     # cycle 1: time [1, 5)
     # cycle 2: time [5, 9)
@@ -1390,13 +1424,13 @@ def test_fold_v2():
     fld = lc.fold(period=1)
     fld2 = lc.fold(period=1 * u.day)
     assert_array_equal(fld.phase, fld2.phase)
-    assert isinstance(fld.time, TimeDelta)
+    assert isinstance(fld.phase, TimeDelta)
     fld.plot_river()
     plt.close()
 
     # Does phase normalization work?
     fld = lc.fold(period=1, normalize_phase=True)
-    assert isinstance(fld.time, u.Quantity)
+    assert isinstance(fld.phase, u.Quantity)
     fld.plot_river()
     plt.close()