Repository: ToucanToco/fastexcel (language: python)

Issue:
`column_names` are taken partially when `use_columns` does not include all columns
Given this worksheet data (without any empty area):

| A   | B   | C   |
| --- | --- | --- |
| 21  | 22  | 23  |
| 31  | 32  | 33  |
| 41  | 42  | 43  |

The following code:

```python
import fastexcel as fe


file = r'<path to file>'
params = {'idx_or_name': 0, 'header_row': None, 'skip_rows': 1, 'use_columns': [1, 2], 'column_names': ['Col B', 'Col C']}

print(fe.read_excel(file).load_sheet(**params).to_polars())
```

Outputs:

```python
shape: (3, 2)
┌───────┬──────────────┐
│ Col C ┆ __UNNAMED__2 │
│ ---   ┆ ---          │
│ f64   ┆ f64          │
╞═══════╪══════════════╡
│ 22.0  ┆ 23.0         │
│ 32.0  ┆ 33.0         │
│ 42.0  ┆ 43.0         │
└───────┴──────────────┘
```

Expected output:

```python
shape: (3, 2)
┌───────┬───────┐
│ Col B ┆ Col C │
│ ---   ┆ ---   │
│ f64   ┆ f64   │
╞═══════╪═══════╡
│ 22.0  ┆ 23.0  │
│ 32.0  ┆ 33.0  │
│ 42.0  ┆ 43.0  │
└───────┴───────┘
```


Discussion / hints:
Hi @wikiped this is actually the expected behaviour: `column_names` is used to specify the name of the columns in the resulting DataFrame. Since there are three columns and only two names were provided, the third name is generated based on the column index.: https://fastexcel.toucantoco.dev/fastexcel.html#ExcelReader.load_sheet

Achieving the result you want is possible with the `use_columns` parameter:

```py
import fastexcel as fe

file = 'repro_214.xlsx'
params = {'idx_or_name': 0, 'use_columns': ['B', 'C']}

print(fe.read_excel(file).load_sheet(**params).to_polars())
```

Will print
```
shape: (3, 2)
┌──────┬──────┐
│ B    ┆ C    │
│ ---  ┆ ---  │
│ f64  ┆ f64  │
╞══════╪══════╡
│ 22.0 ┆ 23.0 │
│ 32.0 ┆ 33.0 │
│ 42.0 ┆ 43.0 │
└──────┴──────┘
```

However there is indeed a bug when mixing `use_columns` and `column_names`, so thanks for the report!

```py
import fastexcel as fe


file = 'repro_214.xlsx'
params = {'idx_or_name': 0, 'header_row': None, 'skip_rows': 1, 'column_names': ['Col B', 'Col C'], 'use_columns': [1, 2]}

print(fe.read_excel(file).load_sheet(**params).to_polars())
```
Returns
```
shape: (3, 2)
┌───────┬──────────────┐
│ Col C ┆ __UNNAMED__2 │
│ ---   ┆ ---          │
│ f64   ┆ f64          │
╞═══════╪══════════════╡
│ 22.0  ┆ 23.0         │
│ 32.0  ┆ 33.0         │
│ 42.0  ┆ 43.0         │
└───────┴──────────────┘
```
And the expected result would be 
```
shape: (3, 2)
┌───────┬───────┐
│ Col B ┆ Col C │
│ ---   ┆ ---   │
│ f64   ┆ f64   │
╞═══════╪═══════╡
│ 22.0  ┆ 23.0  │
│ 32.0  ┆ 33.0  │
│ 42.0  ┆ 43.0  │
└───────┴───────┘
```
[repro_214.xlsx](https://github.com/ToucanToco/fastexcel/files/14786952/repro_214.xlsx)

Thank you for the feedback, I realized now that I forgot to paste the significant part of the parameters used: `'use_columns': [1, 2]`, which is, as you noted, part of the reason for the raising the issue.

Fix patch:
diff --git a/src/types/python/excelsheet/column_info.rs b/src/types/python/excelsheet/column_info.rs
index d26882e..8e502ac 100644
--- a/src/types/python/excelsheet/column_info.rs
+++ b/src/types/python/excelsheet/column_info.rs
@@ -182,7 +182,7 @@ impl ColumnInfo {
     }
 
     pub fn __repr__(&self) -> String {
-        format!("ColumnInfo<name=\"{name}\", index={index}, dtype=\"{dtype}\", dtype_from=\"{dtype_from}\", column_name_from=\"{column_name_from}\" >", name=self.name, index=self.index, dtype=self.dtype.to_string(), dtype_from=self.dtype_from.to_string(), column_name_from=self.column_name_from.to_string())
+        format!("ColumnInfo(name=\"{name}\", index={index}, dtype=\"{dtype}\", dtype_from=\"{dtype_from}\", column_name_from=\"{column_name_from}\" )", name=self.name, index=self.index, dtype=self.dtype.to_string(), dtype_from=self.dtype_from.to_string(), column_name_from=self.column_name_from.to_string())
     }
 
     pub fn __eq__(&self, other: &Self) -> bool {
diff --git a/src/types/python/excelsheet/mod.rs b/src/types/python/excelsheet/mod.rs
index 4351935..4792891 100644
--- a/src/types/python/excelsheet/mod.rs
+++ b/src/types/python/excelsheet/mod.rs
@@ -326,9 +326,6 @@ impl ExcelSheet {
         selected_columns: SelectedColumns,
         dtypes: Option<DTypeMap>,
     ) -> FastExcelResult<Self> {
-        // Ensuring dtypes are compatible with selected columns
-        // Self::validate_dtypes_and_selected_columns(&selected_columns, &dtypes)?;
-
         let mut sheet = ExcelSheet {
             name,
             header,
@@ -344,7 +341,7 @@ impl ExcelSheet {
             selected_columns: Vec::with_capacity(0),
         };
 
-        let available_columns_info = sheet.get_available_columns_info();
+        let available_columns_info = sheet.get_available_columns_info(&selected_columns)?;
 
         let mut aliased_available_columns = Vec::with_capacity(available_columns_info.len());
 
@@ -379,10 +376,13 @@ impl ExcelSheet {
         Ok(sheet)
     }
 
-    fn get_available_columns_info(&self) -> Vec<ColumnInfoBuilder> {
+    fn get_available_columns_info(
+        &self,
+        selected_columns: &SelectedColumns,
+    ) -> FastExcelResult<Vec<ColumnInfoBuilder>> {
         let width = self.data.width();
         match &self.header {
-            Header::None => (0..width)
+            Header::None => Ok((0..width)
                 .map(|col_idx| {
                     ColumnInfoBuilder::new(
                         format!("__UNNAMED__{col_idx}"),
@@ -390,8 +390,8 @@ impl ExcelSheet {
                         ColumnNameFrom::Generated,
                     )
                 })
-                .collect(),
-            Header::At(row_idx) => (0..width)
+                .collect()),
+            Header::At(row_idx) => Ok((0..width)
                 .map(|col_idx| {
                     self.data
                         .get((*row_idx, col_idx))
@@ -407,23 +407,73 @@ impl ExcelSheet {
                             )
                         })
                 })
-                .collect(),
+                .collect()),
             Header::With(names) => {
-                let nameless_start_idx = names.len();
-                names
-                    .iter()
-                    .enumerate()
-                    .map(|(col_idx, name)| {
-                        ColumnInfoBuilder::new(name.to_owned(), col_idx, ColumnNameFrom::Provided)
-                    })
-                    .chain((nameless_start_idx..width).map(|col_idx| {
-                        ColumnInfoBuilder::new(
-                            format!("__UNNAMED__{col_idx}"),
-                            col_idx,
-                            ColumnNameFrom::Generated,
+                if let SelectedColumns::Selection(column_selection) = selected_columns {
+                    if column_selection.len() != names.len() {
+                        return Err(FastExcelErrorKind::InvalidParameters(
+                            "column_names and use_columns must have the same length".to_string(),
                         )
-                    }))
-                    .collect()
+                        .into());
+                    }
+                    let selected_indices = column_selection
+                        .iter()
+                        .map(|idx_or_name| {
+                            match idx_or_name {
+                        IdxOrName::Idx(idx) => Ok(*idx),
+                        IdxOrName::Name(name) => Err(FastExcelErrorKind::InvalidParameters(
+                            format!("use_columns can only contain integers when used with columns_names, got \"{name}\"")
+                        )
+                        .into()),
+                    }
+                        })
+                        .collect::<FastExcelResult<Vec<_>>>()?;
+
+                    Ok((0..width)
+                        .map(|col_idx| {
+                            let provided_name_opt = if let Some(pos_in_names) =
+                                selected_indices.iter().position(|idx| idx == &col_idx)
+                            {
+                                names.get(pos_in_names).cloned()
+                            } else {
+                                None
+                            };
+
+                            match provided_name_opt {
+                                Some(provided_name) => ColumnInfoBuilder::new(
+                                    provided_name,
+                                    col_idx,
+                                    ColumnNameFrom::Provided,
+                                ),
+                                None => ColumnInfoBuilder::new(
+                                    format!("__UNNAMED__{col_idx}"),
+                                    col_idx,
+                                    ColumnNameFrom::Generated,
+                                ),
+                            }
+                        })
+                        .collect())
+                } else {
+                    let nameless_start_idx = names.len();
+                    Ok(names
+                        .iter()
+                        .enumerate()
+                        .map(|(col_idx, name)| {
+                            ColumnInfoBuilder::new(
+                                name.to_owned(),
+                                col_idx,
+                                ColumnNameFrom::Provided,
+                            )
+                        })
+                        .chain((nameless_start_idx..width).map(|col_idx| {
+                            ColumnInfoBuilder::new(
+                                format!("__UNNAMED__{col_idx}"),
+                                col_idx,
+                                ColumnNameFrom::Generated,
+                            )
+                        }))
+                        .collect())
+                }
             }
         }
     }


Test patch:
diff --git a/python/tests/test_column_selection.py b/python/tests/test_column_selection.py
index ce1dca5..14a5387 100644
--- a/python/tests/test_column_selection.py
+++ b/python/tests/test_column_selection.py
@@ -240,24 +240,28 @@ def test_single_sheet_with_unnamed_columns_and_pagination_and_column_names(
     use_columns_idx = [0, 2, 3]
     expected: dict[str, list[Any]] = {
         "col0": [2.0, 3.0],
-        "col2": ["hello", "world"],
-        "col3": [-5.0, -6.0],
+        "col1": ["hello", "world"],
+        "col2": [-5.0, -6.0],
     }
-    column_names = [f"col{i}" for i in range(5)]
+    column_names = [f"col{i}" for i in range(3)]
+    expected_columns_names = ["col0", "__UNNAMED__1", "col1", "col2", "__UNNAMED__4"]
 
     # skipping the header row only
-    sheet = excel_reader_single_sheet_with_unnamed_columns.load_sheet(
-        "With unnamed columns", use_columns=use_columns_str, skip_rows=1, column_names=column_names
-    )
-    assert [col.name for col in sheet.available_columns] == column_names
-
-    pd_assert_frame_equal(sheet.to_pandas(), pd.DataFrame(expected))
-    pl_assert_frame_equal(sheet.to_polars(), pl.DataFrame(expected))
+    with pytest.raises(
+        fastexcel.InvalidParametersError,
+        match='use_columns can only contain integers when used with columns_names, got "col0"',
+    ):
+        excel_reader_single_sheet_with_unnamed_columns.load_sheet(
+            "With unnamed columns",
+            use_columns=use_columns_str,
+            skip_rows=1,
+            column_names=column_names,
+        )
 
     sheet = excel_reader_single_sheet_with_unnamed_columns.load_sheet(
         "With unnamed columns", use_columns=use_columns_idx, skip_rows=1, column_names=column_names
     )
-    assert [col.name for col in sheet.available_columns] == column_names
+    assert [col.name for col in sheet.available_columns] == expected_columns_names
 
     pd_assert_frame_equal(sheet.to_pandas(), pd.DataFrame(expected))
     pl_assert_frame_equal(sheet.to_polars(), pl.DataFrame(expected))
@@ -265,18 +269,10 @@ def test_single_sheet_with_unnamed_columns_and_pagination_and_column_names(
     # skipping the header row + first data row
     expected_first_row_skipped = {k: v[1:] for k, v in expected.items()}
 
-    sheet = excel_reader_single_sheet_with_unnamed_columns.load_sheet(
-        "With unnamed columns", use_columns=use_columns_str, skip_rows=2, column_names=column_names
-    )
-    assert [col.name for col in sheet.available_columns] == column_names
-
-    pd_assert_frame_equal(sheet.to_pandas(), pd.DataFrame(expected_first_row_skipped))
-    pl_assert_frame_equal(sheet.to_polars(), pl.DataFrame(expected_first_row_skipped))
-
     sheet = excel_reader_single_sheet_with_unnamed_columns.load_sheet(
         "With unnamed columns", use_columns=use_columns_idx, skip_rows=2, column_names=column_names
     )
-    assert [col.name for col in sheet.available_columns] == column_names
+    assert [col.name for col in sheet.available_columns] == expected_columns_names
 
     pd_assert_frame_equal(sheet.to_pandas(), pd.DataFrame(expected_first_row_skipped))
     pl_assert_frame_equal(sheet.to_polars(), pl.DataFrame(expected_first_row_skipped))
diff --git a/python/tests/test_use_columns.py b/python/tests/test_use_columns.py
new file mode 100644
index 0000000..4115c08
--- /dev/null
+++ b/python/tests/test_use_columns.py
@@ -0,0 +1,74 @@
+from __future__ import annotations
+
+import fastexcel
+import pandas as pd
+import polars as pl
+from pandas.testing import assert_frame_equal as pd_assert_frame_equal
+from polars.testing import assert_frame_equal as pl_assert_frame_equal
+from utils import path_for_fixture
+
+
+def test_use_columns_with_use_columns() -> None:
+    excel_reader = fastexcel.read_excel(path_for_fixture("fixture-single-sheet-with-types.xlsx"))
+
+    sheet = excel_reader.load_sheet(
+        0,
+        use_columns=[1, 2],
+        header_row=None,
+        skip_rows=1,
+        column_names=["bools_renamed", "dates_renamed"],
+    )
+
+    assert sheet.available_columns == [
+        fastexcel.ColumnInfo(
+            name="__UNNAMED__0",
+            column_name_from="generated",
+            index=0,
+            dtype="float",
+            dtype_from="guessed",
+        ),
+        fastexcel.ColumnInfo(
+            name="bools_renamed",
+            index=1,
+            dtype="boolean",
+            dtype_from="guessed",
+            column_name_from="provided",
+        ),
+        fastexcel.ColumnInfo(
+            name="dates_renamed",
+            index=2,
+            dtype="datetime",
+            dtype_from="guessed",
+            column_name_from="provided",
+        ),
+        fastexcel.ColumnInfo(
+            name="__UNNAMED__3",
+            index=3,
+            dtype="float",
+            dtype_from="guessed",
+            column_name_from="generated",
+        ),
+    ]
+
+    pd_assert_frame_equal(
+        sheet.to_pandas(),
+        pd.DataFrame(
+            {
+                "bools_renamed": [True, False, True],
+                "dates_renamed": pd.Series([pd.Timestamp("2022-03-02 05:43:04")] * 3).astype(
+                    "datetime64[ms]"
+                ),
+            }
+        ),
+    )
+    pl_assert_frame_equal(
+        sheet.to_polars(),
+        pl.DataFrame(
+            {
+                "bools_renamed": [True, False, True],
+                "dates_renamed": ["2022-03-02 05:43:04"] * 3,
+            }
+        ).with_columns(
+            pl.col("dates_renamed").str.strptime(pl.Datetime, "%F %T").dt.cast_time_unit("ms")
+        ),
+    )