Repository: feder-observatory/stellarphot

Issue:
Creating `PhotometryData` fails if the pasbband_map new passband names are longer than the longest of the original passband names
Consider this setup:

+ The input data to `PhotometryData` has a `passband` column whose contents are `['i']`.
+ The 1passband_map` is `{'i': 'SI'}`

This will issue a warning and fail to update the filter value because the original dtype of the column is based on the length of the longest string in the column, in this case `U1` so unicode, 1 character. Settings the value to a longer string, e.g. `SI`, results in truncation of the value to one character, in this case setting the value to `S`.

The warning is:

```
E           astropy.table.column.StringTruncateWarning: truncated right side string(s) longer than 1 character(s) during assignment
```

Test for this and fix is incoming.

Discussion:

Patch:
diff --git a/.readthedocs.yaml b/.readthedocs.yaml
index e8b3e1d..f67b32a 100644
--- a/.readthedocs.yaml
+++ b/.readthedocs.yaml
@@ -14,7 +14,7 @@ build:
   apt_packages:
     - graphviz
   tools:
-    python: "mambaforge-22.9"
+    python: "mambaforge-latest"
 
 python:
   install:
diff --git a/.rtd-environment.yml b/.rtd-environment.yml
index b88df5f..f6dbbc3 100644
--- a/.rtd-environment.yml
+++ b/.rtd-environment.yml
@@ -3,6 +3,7 @@ channels:
     - defaults
 dependencies:
     - python>=3.10
+    - pip
     - astropy >=5
     - astroquery
     - ccdproc
@@ -15,9 +16,11 @@ dependencies:
     - gatspy
     - pyyaml
     - astrowidgets
-    - pydantic<2
+    - pydantic>=2
     - reducer
     - reproject
+    - sphinx
     - sphinx-astropy
+    - sphinx-automodapi=0.16
     - pip:
         - ipyautoui
diff --git a/stellarphot/core.py b/stellarphot/core.py
index 3e125f9..d9b9b22 100644
--- a/stellarphot/core.py
+++ b/stellarphot/core.py
@@ -166,9 +166,15 @@ class BaseEnhancedTable(QTable):
     def _update_passbands(self):
         # Converts filter names in filter column to AAVSO standard names
         # Assumes _passband_map is in namespace.
-        for orig_pb, aavso_pb in self._passband_map.items():
-            mask = self["passband"] == orig_pb
-            self["passband"][mask] = aavso_pb
+
+        # Create a list of new passband names instead of trying to change names in place
+        # in case any of the new names are longer than the longest of the old names.
+        # If that happens, astropy by default just truncates the names.
+        new_filter_name = [
+            self._passband_map[orig_pb] if orig_pb in self._passband_map else orig_pb
+            for orig_pb in self["passband"]
+        ]
+        self["passband"] = new_filter_name
 
     def clean(self, remove_rows_with_mask=False, **other_restrictions):
         """
diff --git a/stellarphot/io/tess.py b/stellarphot/io/tess.py
index 8994734..d90d127 100644
--- a/stellarphot/io/tess.py
+++ b/stellarphot/io/tess.py
@@ -457,7 +457,7 @@ class TessTargetFile:
     def __post_init__(self):
         self.aperture_server = GAIA_APERTURE_SERVER
         if not self.file:
-            # Be sure not to delete the file -- otherwise, one windows,
+            # Be sure not to delete the file -- otherwise, on windows,
             # the file is deleted immediately as far as I can tell.
             self.file = NamedTemporaryFile(delete=False)
         self._path = Path(self.file.name)


Test patch:
diff --git a/stellarphot/io/tests/test_tess_submission.py b/stellarphot/io/tests/test_tess_submission.py
index 6a961ba..ae9ed0d 100644
--- a/stellarphot/io/tests/test_tess_submission.py
+++ b/stellarphot/io/tests/test_tess_submission.py
@@ -3,6 +3,7 @@ import warnings
 
 import pytest
 from astropy.coordinates import SkyCoord
+from requests import ConnectionError
 
 from stellarphot.io.tess import TessSubmission, TessTargetFile
 
@@ -80,27 +81,40 @@ def test_target_file():
     # object
 
     tic_742648307 = SkyCoord(ra=104.733225, dec=49.968739, unit="degree")
-
-    tess_target = TessTargetFile(tic_742648307, magnitude=12, depth=10)
-
-    # Check that the first thing in the list is the tick object
-    check_coords = SkyCoord(
-        ra=tess_target.table["RA"][0],
-        dec=tess_target.table["Dec"][0],
-        unit=("hour", "degree"),
-    )
-    assert tic_742648307.separation(check_coords).arcsecond < 1
-
-    # On windows the temporary file cannot be deleted because if it is
-    # then the file immediately vanished. That means we get a warning
-    # about having an open file handle. We can ignore that warning
-    # because we don't care about it here.
-
-    # The warning doesn't get generated until the last reference to
-    # the object is deleted, so we need to do that here instead of
-    # letting it happen at the end of the test.
-    with warnings.catch_warnings():
-        warnings.filterwarnings(
-            "ignore", message="unclosed file", category=ResourceWarning
+    server_down = False
+
+    try:
+        tess_target = TessTargetFile(tic_742648307, magnitude=12, depth=10)
+    except ConnectionError:
+        server_down = True
+        tess_target = None  # Assure tess_target is defined so that we can delete it
+    else:
+        # Check that the first thing in the list is the tick object
+        check_coords = SkyCoord(
+            ra=tess_target.table["RA"][0],
+            dec=tess_target.table["Dec"][0],
+            unit=("hour", "degree"),
         )
-        del tess_target
+        assert tic_742648307.separation(check_coords).arcsecond < 1
+    finally:
+        # Make sure that regardless of whether the server is down we delete
+        # the temporary file that gets created.
+
+        # On windows the temporary file cannot be deleted because if it is
+        # then the file immediately vanished. That means we get a warning
+        # about having an open file handle. We can ignore that warning
+        # because we don't care about it here.
+
+        # The warning doesn't get generated until the last reference to
+        # the object is deleted, so we need to do that here instead of
+        # letting it happen at the end of the test.
+        with warnings.catch_warnings():
+            warnings.filterwarnings(
+                "ignore", message="unclosed file", category=ResourceWarning
+            )
+            del tess_target
+
+        if server_down:
+            # The server at University of Louisville is down sometimes, so
+            # xfail this test when that happens
+            pytest.xfail("TESS/gaia Server down")
diff --git a/stellarphot/tests/test_core.py b/stellarphot/tests/test_core.py
index ca03cc5..6f6b095 100644
--- a/stellarphot/tests/test_core.py
+++ b/stellarphot/tests/test_core.py
@@ -506,6 +506,59 @@ def test_photometry_data():
     assert (phot_data["bjd"][0].value - 2459910.775405664) * 86400 < 0.05
 
 
+def test_photometry_data_short_filter_name():
+    # Regression test for #279.
+    # If the final passband name in the passband_map is longer than the
+    # longest passband name in the input data, the updated passband was
+    # truncated. That should not happen.
+
+    data = testphot_clean.copy()
+    # Delete the original passband column, just in case it is longer than
+    # the new passband name.
+    del data["passband"]
+
+    # Make a new 1-character column for passband
+    data["passband"] = "i"
+
+    phot_data = PhotometryData(
+        observatory=feder_obs,
+        camera=feder_cg_16m,
+        passband_map={"i": "SI"},
+        input_data=data,
+    )
+
+    # Make sure the passband name is not truncated.
+    assert phot_data["passband"][0] == "SI"
+
+
+def test_photometry_data_filter_name_map_preserves_original_names():
+    # If a passband is not in the passband map it should be preserved.
+
+    data = testphot_clean.copy()
+    # Delete the original passband column
+    del data["passband"]
+
+    # Make a new 1-character column for passband
+    data["passband"] = "i"
+
+    # Add a second (identical) row
+    data.add_row(data[0])
+
+    # Change the first entry to a value not in the passband map
+    data["passband"][0] = "Q"
+
+    phot_data = PhotometryData(
+        observatory=feder_obs,
+        camera=feder_cg_16m,
+        passband_map={"i": "SI"},
+        input_data=data,
+    )
+
+    # Make sure the passband names are correct
+    assert phot_data["passband"][0] == "Q"
+    assert phot_data["passband"][1] == "SI"
+
+
 def test_photometry_roundtrip_ecsv(tmp_path):
     # Check that we can save the test data to ECSV and restore it
     file_path = tmp_path / "test_photometry.ecsv"