Repository: lightkurve/lightkurve

Issue:
`LightCurve.select_flux()` would cause inconsistent units between flux and flux_err in some cases, e.g., `flux_bkg` from GSFC-ELEANOR-LITE
<!-- Fill in the information below before opening an issue. -->

#### Problem description
<!-- Provide a clear and concise description of the issue. -->

`LightCurve.select_flux()` would cause inconsistent units between flux and flux_err in some cases, e.g., `flux_bkg` from GSFC-ELEANOR-LITE. The inconsistency would lead to errors in some usage.


#### Example
<!-- Provide a link or minimal code snippet that demonstrates the issue. -->
```python
import lightkurve as lk

lc = lk.search_lightcurve("TIC 336732616", author="GSFC-ELEANOR-LITE", sector=1)[0].download()
print("Units of flux, flux_err:", lc.flux.unit, ",", lc.flux_err.unit, "; origin:", lc.flux_origin);

lc = lc.select_flux("flux_bkg");
print("Units of flux, flux_err:", lc.flux.unit, ",", lc.flux_err.unit, "; origin:", lc.flux_origin);

lc = lc["time", "flux", "flux_err"]  # ValueError
```

The output is:

```python
Units of flux, flux_err: electron / s , electron / s ; origin: corr_flux
Units of flux, flux_err: % , electron / s ; origin: flux_bkg
```

with a `ValueError`
```python
ValueError                                Traceback (most recent call last)
Cell In[1178], line 7
      4 lc = lc.select_flux("flux_bkg");
      5 print("Units of flux, flux_err:", lc.flux.unit, ",", lc.flux_err.unit, "; origin:", lc.flux_origin);
----> 7 lc = lc["time", "flux", "flux_err"]

File C:\pkg\_winNonPortables\miniforge3\envs\my_lk_plus\lib\site-packages\astropy\timeseries\sampled.py:293, in TimeSeries.__getitem__(self, item)
    289         out._groups = groups.TableGroups(
    290             out, indices=self.groups._indices, keys=self.groups._keys
    291         )
    292         return out
--> 293 return super().__getitem__(item)

File C:\pkg\_winNonPortables\miniforge3\envs\my_lk_plus\lib\site-packages\astropy\table\table.py:2070, in Table.__getitem__(self, item)
   2068     return self.Row(self, item.item())
   2069 elif self._is_list_or_tuple_of_str(item):
-> 2070     out = self.__class__(
   2071         [self[x] for x in item], copy_indices=self._copy_indices
   2072     )
   2073     out._groups = groups.TableGroups(
   2074         out, indices=self.groups._indices, keys=self.groups._keys
   2075     )
   2076     out.meta = self.meta.copy()  # Shallow copy for meta

File C:\pkg\_winNonPortables\miniforge3\envs\my_lk_plus\lib\site-packages\lightkurve\lightcurve.py:400, in LightCurve.__init__(self, data, time, flux, flux_err, *args, **kwargs)
    398 # Ensure flux and flux_err have the same units
    399 if self["flux"].unit != self["flux_err"].unit:
--> 400     raise ValueError("flux and flux_err must have the same units")
    402 self._new_attributes_relax = False
    403 self._required_columns_relax = False

ValueError: flux and flux_err must have the same units
```

#### Expected behavior
<!-- Describe the behavior you expected and how it differs from the behavior observed in the example. -->
`select_flux()` should not cause inconsistency between flux / flux_err.

#### Environment

-  platform (e.g. Linux, OSX, Windows): Windows
-  lightkurve version (e.g. 1.0b6): 2.5.0
-  installation method (e.g. pip, conda, source): conda

Hints from issue comments:
@orionlee: In general, the inconsistency would happen when:
1. the units between current `flux` and the new flux being selected is different, and
2. the new flux has no `_err` column.


Solution patch:
diff --git a/CHANGES.rst b/CHANGES.rst
index ed1e86b70..b9272dae1 100644
--- a/CHANGES.rst
+++ b/CHANGES.rst
@@ -1,8 +1,12 @@
 2.5.1  (unreleased)
 =====================
-- Fixed pixel to world coordinate transformation in ``TargetPixelFile.get_coordinates()`` 
-  in line 488 ("ra, dec = w.wcs_pix2world(X.ravel(), Y.ravel(), 1)"), where for consistency with 
-  Gaia the origin should be 0 instead of 1.
+
+- Fixed pixel to world coordinate transformation in ``TargetPixelFile.get_coordinates()``
+  in line 488 ("ra, dec = w.wcs_pix2world(X.ravel(), Y.ravel(), 1)"), where for consistency with
+  Gaia the origin should be 0 instead of 1. [#1465]
+- Fixed ``LightCurve.select_flux()`` in edge cases, where the unit of
+  the new ``flux`` column is different from that of the ``flux_err`` column. [#1467]
+
 
 2.5.0 (2024-08-29)
 =====================
diff --git a/src/lightkurve/lightcurve.py b/src/lightkurve/lightcurve.py
index dbc92f842..bc90aa22d 100644
--- a/src/lightkurve/lightcurve.py
+++ b/src/lightkurve/lightcurve.py
@@ -559,7 +559,21 @@ def select_flux(self, flux_column, flux_err_column=None):
             if flux_err_column in lc.columns:
                 lc["flux_err"] = lc[flux_err_column]
             else:
-                lc["flux_err"][:] = np.nan
+                # fill in a dummy all-nan flux_err column
+                # ensure the unit of new flux_err is consistent with that of flux.
+                flux_err_col_vals = np.full(lc["flux"].shape, np.nan)
+                if lc["flux"].unit is not None:
+                    flux_err_col_vals = flux_err_col_vals * lc["flux"].unit
+                lc["flux_err"] = flux_err_col_vals
+
+        # Ensure resulting flux / flux_err have the same
+        # Do the check here after the columns are selected so as to uniformly handle
+        # different cases.
+        if lc["flux"].unit != lc["flux_err"].unit:
+            raise ValueError(
+                f"Columns '{flux_column}' and '{flux_err_column}' have different units: "
+                f"{lc.flux.unit} and {lc.flux_err.unit} respectively."
+            )
 
         lc.meta['FLUX_ORIGIN'] = flux_column
         normalized_new_flux = lc["flux"].unit is None or lc["flux"].unit is u.dimensionless_unscaled


Test patch:
diff --git a/tests/io/test_eleanor.py b/tests/io/test_eleanor.py
index 7a49cb736..3e8126b71 100644
--- a/tests/io/test_eleanor.py
+++ b/tests/io/test_eleanor.py
@@ -3,7 +3,7 @@
 from astropy.io import fits
 from astropy.utils.data import get_pkg_data_filename
 import numpy as np
-from numpy.testing import assert_array_equal
+from numpy.testing import assert_array_equal, assert_equal
 
 from lightkurve import search_lightcurve
 from lightkurve.io.eleanor import read_eleanor_lightcurve
@@ -32,6 +32,10 @@ def test_gsfc_eleanor_lite():
         assert not (lc["quality"] & (2**17 | 2**18)).any()
         assert np.issubdtype(lc["cadenceno"].dtype, np.integer)
 
+        # https://github.com/lightkurve/lightkurve/issues/1467
+        lc = lc.select_flux("flux_bkg")
+        assert_equal(lc["flux_err"].unit, lc["flux"].unit)
+
 
 @pytest.mark.parametrize(
     "url",
diff --git a/tests/test_lightcurve.py b/tests/test_lightcurve.py
index f8a0adcf9..43222b509 100644
--- a/tests/test_lightcurve.py
+++ b/tests/test_lightcurve.py
@@ -1947,6 +1947,10 @@ def test_select_flux():
                           'newflux_err': [7, 8, 9] * u_e_s,
                           'newflux_n1': [0.9, 1, 1.1] * u.dimensionless_unscaled,  # normalized, unitless
                           'newflux_n2': [0.9, 1, 1.1],  # normalized, no unit
+                          'newflux_n3': [4, 5, 6] * u_e_s,  # case flux and _err have different units
+                          'newflux_n3_err': [1, 2, 3] * u.percent,
+                          'newflux_n4': [4, 5, 6] * u_e_s,  # case flux and _err have different units
+                          'newflux_n4_err': [.01, .02, .03],  # normalized, no unit
                           },
                           )
     # Can we set flux to newflux?
@@ -1960,6 +1964,15 @@ def test_select_flux():
     assert all(lc.select_flux("newflux", flux_err_column="newflux").flux_err == lc.newflux)
     # ensure flux_err in the new lc is nan if the origin does not have it
     assert all(np.isnan(lc.select_flux("newflux_n1")["flux_err"]))
+    assert_equal(  # https://github.com/lightkurve/lightkurve/issues/1467
+        lc.select_flux("newflux_n1")["flux_err"].unit, lc.select_flux("newflux_n1")["flux"].unit,
+        "The unit of the all-nan flux_err should be the same as that of flux [#1467]"
+    )
+    # Do inconsistent units in the selected columns raise a ValueError? [issue 1467]
+    with pytest.raises(ValueError, match="different units"):
+        lc.select_flux("newflux_n3")
+    with pytest.raises(ValueError, match="different units"):
+        lc.select_flux("newflux_n4")
     # Do invalid column names raise a ValueError?
     with pytest.raises(ValueError):
         lc.select_flux("doesnotexist")