Reorder columns before returning the table

Author: juanmcloaiza

astroquery/eso/core.py

@@ -38,7 +38,9 @@
 from ..query import QueryWithLogin
 from ..utils import schema
 from .utils import py2adql, _split_str_as_list_of_str, \
-    adql_sanitize_val, are_coords_valid, reorder_columns
+    adql_sanitize_val, are_coords_valid, reorder_columns, \
+    DEFAULT_LEAD_COLS_PHASE3, DEFAULT_LEAD_COLS_RAW
+
 
 __doctest_skip__ = ['EsoClass.*']
 
@@ -301,7 +303,6 @@ def query_tap_service(self,
         table_to_return = None
         tap_service = self._tap_service(authenticated)
         table_to_return = self._try_download_pyvo_table(query_str, tap_service)
-        table_to_return = reorder_columns(table_to_return)
         return table_to_return
 
     @unlimited_max_rec
@@ -522,7 +523,7 @@ def query_surveys(
         _ = open_form, cache  # make explicit that we are aware these arguments are unused
         c = column_filters if column_filters else {}
         kwargs = {**kwargs, **c}
-        return self._query_on_allowed_values(table_name=EsoNames.phase3_table,
+        t = self._query_on_allowed_values(table_name=EsoNames.phase3_table,
                                              column_name=EsoNames.phase3_surveys_column,
                                              allowed_values=surveys,
                                              cone_ra=cone_ra,
@@ -535,6 +536,8 @@ def query_surveys(
                                              print_help=help,
                                              authenticated=authenticated,
                                              **kwargs)
+        t = reorder_columns(t, DEFAULT_LEAD_COLS_PHASE3)
+        return t
 
     @deprecated_renamed_argument(('open_form', 'cache'), (None, None),
                                  since=['0.4.11', '0.4.11'])
@@ -615,7 +618,7 @@ def query_main(
         _ = open_form, cache  # make explicit that we are aware these arguments are unused
         c = column_filters if column_filters else {}
         kwargs = {**kwargs, **c}
-        return self._query_on_allowed_values(table_name=EsoNames.raw_table,
+        t = self._query_on_allowed_values(table_name=EsoNames.raw_table,
                                              column_name=EsoNames.raw_instruments_column,
                                              allowed_values=instruments,
                                              cone_ra=cone_ra,
@@ -628,6 +631,8 @@ def query_main(
                                              print_help=help,
                                              authenticated=authenticated,
                                              **kwargs)
+        t = reorder_columns(t, DEFAULT_LEAD_COLS_RAW)
+        return t
 
     @deprecated_renamed_argument(('open_form', 'cache'), (None, None),
                                  since=['0.4.11', '0.4.11'])
@@ -707,19 +712,21 @@ def query_instrument(
         _ = open_form, cache  # make explicit that we are aware these arguments are unused
         c = column_filters if column_filters else {}
         kwargs = {**kwargs, **c}
-        return self._query_on_allowed_values(table_name=EsoNames.ist_table(instrument),
-                                             column_name=None,
-                                             allowed_values=None,
-                                             cone_ra=cone_ra,
-                                             cone_dec=cone_dec,
-                                             cone_radius=cone_radius,
-                                             columns=columns,
-                                             top=top,
-                                             count_only=count_only,
-                                             query_str_only=query_str_only,
-                                             print_help=help,
-                                             authenticated=authenticated,
-                                             **kwargs)
+        t = self._query_on_allowed_values(table_name=EsoNames.ist_table(instrument),
+                                          column_name=None,
+                                          allowed_values=None,
+                                          cone_ra=cone_ra,
+                                          cone_dec=cone_dec,
+                                          cone_radius=cone_radius,
+                                          columns=columns,
+                                          top=top,
+                                          count_only=count_only,
+                                          query_str_only=query_str_only,
+                                          print_help=help,
+                                          authenticated=authenticated,
+                                          **kwargs)
+        t = reorder_columns(t, DEFAULT_LEAD_COLS_RAW)
+        return t
 
     def get_headers(self, product_ids, *, cache=True):
         """

astroquery/eso/tests/test_eso.py

@@ -342,14 +342,32 @@ def test_reorder_columns(monkeypatch):
     table2 = reorder_columns(table)
     names_after = table2.colnames[:]
 
+    # the columns we want to change are actually in the table
     assert set(DEFAULT_LEAD_COLS_RAW).issubset(names_before)
     assert set(DEFAULT_LEAD_COLS_RAW).issubset(names_after)
+
+    # no columns are removed nor added
     assert set(names_before) == set(names_after)
-    assert table != table2
-    assert names_before[:5] != names_after[:5]
 
-    for n in table.colnames:
-        assert table[[n]].values_equal(table2[[n]]), n
+    # Column by column, the contents are the same
+    for cname in table.colnames:
+        assert table[[cname]].values_equal(table2[[cname]]), f"Error for {cname}"
+
+    # empty table doesn't cause a crash
+    empty_1 = Table()
+    empty_2 = reorder_columns(empty_1)
+    assert len(empty_1) == 0 and isinstance(empty_1, Table)
+    assert len(empty_2) == 0 and isinstance(empty_1, Table)
+
+    # If the values we're looking for as leading columns, everything stays the same
+    some_table = Table({"x": [1, 2, 3], "y": [4, 5, 6]})
+    same_table = reorder_columns(some_table)
+    assert all(some_table == same_table), "Table with no cols to change fails"
+
+    # If what we pas is not a table, the function has no effect
+    not_a_table_1 = object()
+    not_a_table_2 = reorder_columns(not_a_table_1)
+    assert not_a_table_1 == not_a_table_2
 
 
 def test_py2adql():

astroquery/eso/utils.py

@@ -26,6 +26,9 @@ def reorder_columns(table: Table,
     ['object', 'ra', 'dec', 'dp_id', 'date_obs']
     Returns a table with the columns reordered.
     """
+    if not isinstance(table, Table):
+        return table
+
     leading_cols = leading_cols or DEFAULT_LEAD_COLS_RAW
     first_cols = []
     last_cols = table.colnames[:]

docs/eso/eso.rst

@@ -196,8 +196,8 @@ return two columns: the date of observation and the name of the object.
 
 .. doctest-remote-data::
     >>> table = eso.query_instrument('midi', column_filters={'object':'NGC4151'}, columns=['object', 'date_obs'])
-    >>> t_left = table[table["date_obs"] >= "2008-01-01"]
-    >>> t_2008_2009 = t_left[t_left["date_obs"] <= "2009-05-12"]
+    >>> t_2008 = table[table["date_obs"] >= "2008-01-01"]
+    >>> t_2008_2009 = t_2008[t_2008["date_obs"] <= "2009-05-12"]
     >>> t_2008_2009
     <Table length=196>
     object         date_obs