Repository: ToucanToco/fastexcel (language: python)

Issue:
Support mix of date/timestamp and strings
See https://github.com/ToucanToco/fastexcel/issues/181#issue-2136125477
and https://github.com/ToucanToco/fastexcel/issues/158#issuecomment-1952777001

- [TEST_FASTEXCEL_MIXED_DATA.xlsx](https://github.com/ToucanToco/fastexcel/files/14295251/TEST_FASTEXCEL_MIXED_DATA.xlsx)
  - Same as the prior one, but introducing two additional columns with mixed data types (integer plus string, date plus string, no "NULL" string values this time)

- [example-skip-rows.xlsx](https://github.com/ToucanToco/fastexcel/files/14334373/example-skip-rows.xlsx)
>     0: Could not build schema for sheet Sheet2
>     1: could not figure out column type for following type combination: {Timestamp(Millisecond, None), Utf8}


Discussion / hints:
IMO, we will have to take a design decision here, I see two paths:

1. Either we consider strings to be some kind of uber-type, on which we can fallback in any case meaning we will have to add support for that on the calamine side if we want to avoid double checks for every cell in case `.as_string` returns `None`. Or maybe it should be a new `to_string` method
2. Or we do not do that and consider that data  that gets converted to the arrow format should be uniform (with tolerance for the current int/bool/float implicit conversions). In that case, we would provide a `to_python` method, which would return a simple matrix of python objects. Its type would be `dict[str, list[scalar | None]]` (or `ordereddict` as the columns appear in a certain order) in case we have column names, and `list[list[scalar | None]]` in case we don't. `scalar` would be `bool | int | float | datetime | timedelta | str`

For both cases, it's an opiniated feature, so I'd rather see this in 0.10.0 than in 0.9.1. Thoughts ?
Yup I wanted to have this discussion with you (hence this new issue and my message on Slack 😛)
Let's add that in v0.10.0 and release a new 0.9.1 today.
I guess we could start with the `dtype` enforcement (see https://github.com/ToucanToco/fastexcel/issues/173) and set for a column that fails the type `string` to make it work
So we wouldn't read directly the file but if the error is clear enough, the workaround is easy

Making string the uber-type may or may not be a good idea. If we go with this option I would like some kind of parameter. But let's start with manual dtype declaration. WDYT?
Sorry, missed the slack message, I've disabled all notifications :grimacing: 

Yeah, let's get 0.9.1 out asap.

And I agree, having dtype enforcement is a good start, and it provides a workaround :+1: 
I was about to report this error. For below data I am also getting error as:

```py
Caused by:
    0: Could not build schema for sheet Sheet1
    1: could not figure out column type for following type combination: {Timestamp(Millisecond, None), Float64}
```

![Sample](https://github.com/ToucanToco/fastexcel/assets/110120464/cc513f8b-d79b-42e5-a9e8-ca87146823d0)

My team needs some way to handle bad data in excel files.  We eventually want only dates for a date column but need to check if there are cells with non-dates and report where they occur.  Right now we are using pandas and openpyxl with dtype=object to read excel files with mixed data types matching the original types, so we can still get a dataframe but later check and report an error if there are text cells in a column that should have all dates.  Would be nice if we could use fastexcel (maybe with polars) instead since openpyxl is so slow.
This is planned for v0.10.0. I'll work on it this weekend. @lukapeschke and I will publish the new release in March for sure!
FYI I'm working on #173 , which should allow to enforce a `string` dtype for a given column and convert datetime objects to strings 
Hopefully there will be an option to disallow conversion.  For our use case we ideally need mixed data types in a dataframe (I know pyarrow supports union, but I don't now if it's easy to allow just any type for a column) or at least a list of errors we can trace back to every failed cell by location.

For us it also depends on the type/column whether we need strict enforcement.   We have have had many problems due to incorrect or ambiguous conversion:

- For date columns, we only want to allow date-type cells and no text cells to avoid ambiguity due to different date formats.
- We have some string columns that can have cells that are just numbers.  Excel will by default set the cell types to numbers.  Therefore, we want to allow numbers in these columns and convert to strings.  The problem we've seen is incorrect conversion with pyspark or pandas for numbers.   Very large numbers are converted to scientific notation, which is wrong for our case.   Haven't tested this but floats could also be an issue, e.g. we would need to read a cell showing `198.0`  exactly as `"198.0"`.
- We have some columns where all cells need to be percentages.  So we need these cells to either be excel's percent type or strings with % at the end (this wouldn't normally happen but isn't ambiguous).  But if these columns have a mix of percentages and floats, that's ambiguous/wrong (e.g. `0.2` - we have no way of knowing if this was supposed to be 20% or 0.2%).

For the first two cases I mentioned, for us to be able to use fastexcel instead of openpyxl, we'd need behavior like this:
- We get a mix of arbitrary types matching their original in the dataframe - this is the easier since we can handle the complex checks ourselves later.  Getting the dataframe doesn't error, so we can later map bad data to specific cells in the original file ourselves just once.  This allows us to check/report all errors in one place (we need to report not just type errors but also value errors like "this date is in the future").
- Or we could type e.g. a string cell as string or number and handle the conversion ourselves; we could type a date column as date or string and later error ourselves.  The downside of this is that if there was wildly bad data (e.g. some unforeseen bad type like a date in a string column), we'd now additionally have to parse any errors given when loading the file and cannot report all errors at once.
- Or we could try to rely on fastexcel for all type errors (we type a date column as just date, and fastexcel errors if there are strings).  We'd still have to check/report errors in two places in two different ways though.

For the third problem, I have no idea if there is even anything fastexcel could do about this.  If percentages and floats are both just converted to floats, we have no way of knowing what the original data was.  To address this, we currently have to manually go through every cell for columns that should be percentages with openpyxl and explicitly check the excel type.

Unfortunately, working with excel to correctly detect bad data is complicated.  Not sure if fastexcel can handle our use case, but it would be nice if it could because openpyxl is painfully slow vs. calamine for the large files we have to work with.
@PrettyWood Hello! I am curious about the timeline for when this will be fixed and released. Thanks!
@armgabrielyan Hello! We just released v0.10.0 this morning. It allows to enforce a dtype for a column: https://fastexcel.toucantoco.dev/fastexcel.html#ExcelReader.load_sheet . If you want a column of mixed strings and timestamps as a string, you can now use `dtypes={'my_column': 'string'}` (note that if the `string` dtype is not specified, no conversion will be done implicitly).

@noctuid could you please let us know if this works for you ?

For more advanced mixed dtypes, we're thinking about providing a `to_python` method, which would return a list of lists of python objects. This would be slower and not very efficient memory-wise, as we wouldn't be able to rely on Arrow for data representation, but would allow for greater flexibility. Progress for that is tracked in #197 
@lukapeschke It sounds good. However, we use `fastexcel` as an engine for reading Excel spreadsheets in `polars` and it looks like `polars` does not support this new functionality yet.
@armgabrielyan I believe it should be possible via `read_options`: https://docs.pola.rs/py-polars/html/reference/api/polars.read_excel.html as polars passes it though to `load_sheet`: https://github.com/pola-rs/polars/blob/53f55367d1428b6d4ab51a7b17a8dbf4c003ac43/py-polars/polars/io/spreadsheet/functions.py#L821
@lukapeschke You are correct, it is possible to do this via `read_options`. Thank you so much!
@lukapeschke In our case, the column names or the number of columns is not pre-defined. I was wondering if there is any way to specify the data types for all columns to be `"string"`, for example.
@armgabrielyan No, that's currently not possible, but feel free to open a different issue if you'd like support for that :slightly_smiling_face: 
@lukapeschke Sounds great! Thank you so much for your support!
> could you please let us know if this works for you ?

It doesn't.  Unfortunately `to_python` wouldn't either because of the percentage edge case I mentioned (assuming both excel percentage type and excel float would be converted to python float, there is no way to know what type the original data was).
@noctuid unfortunately, the percentage edge case would require some work in calamine to properly support the excel Percent type, as it is not supported there for now: https://github.com/tafia/calamine/blob/master/src/datatype.rs#L23-L44 .

Are the other points (apart from the percentage case) working for you with v0.10.0  ? If yes, can we close this and create a separate issue for the percentage case ?
With #197, we could handle the other edge cases I mentioned, but I think it would be preferable for us to only use `to_python` as a fallback if there is bad data.

We'd need some control over the coercion to detect bad data.  Will it coerce strings to dates currently?  For example, for a dtype string column, we would want coercion to string from number (or anything).  However, for a date dtype column, we would want an error for a string in that column, even if it could be coerced to a date.
@noctuid  Right now, dates will be coerced to strings only if the dtype of the column is explicitly set to `string`. Implicit coercion will fail, as we did not want to be too smart and return unexpected data. 

I created an issue to explicitly describe that behaviour in the docs: #215 

In the future, it'd be nice to be able to specify how multi-dtype coercion should work

Fix patch:
diff --git a/src/types/dtype.rs b/src/types/dtype.rs
index 1f30306..5be6596 100644
--- a/src/types/dtype.rs
+++ b/src/types/dtype.rs
@@ -187,7 +187,15 @@ fn int_types() -> &'static HashSet<DType> {
 }
 
 fn string_types() -> &'static HashSet<DType> {
-    STRING_TYPES_CELL.get_or_init(|| HashSet::from([DType::Int, DType::Float, DType::String]))
+    STRING_TYPES_CELL.get_or_init(|| {
+        HashSet::from([
+            DType::Int,
+            DType::Float,
+            DType::String,
+            DType::DateTime,
+            DType::Date,
+        ])
+    })
 }
 
 pub(crate) fn get_dtype_for_column<DT: CellType + Debug + DataType>(


Test patch:
diff --git a/python/tests/fixtures/fixture-multi-dtypes-columns.xlsx b/python/tests/fixtures/fixture-multi-dtypes-columns.xlsx
index 3d80b51..4e7211d 100644
Binary files a/python/tests/fixtures/fixture-multi-dtypes-columns.xlsx and b/python/tests/fixtures/fixture-multi-dtypes-columns.xlsx differ
diff --git a/python/tests/test_dtypes.py b/python/tests/test_dtypes.py
index acc5c4e..e155864 100644
--- a/python/tests/test_dtypes.py
+++ b/python/tests/test_dtypes.py
@@ -40,6 +40,7 @@ def expected_data() -> dict[str, list[Any]]:
         "Date": [datetime(2023, 7, 21)] * 9,
         "Details": ["Healthcare"] * 7 + ["Something"] * 2,
         "Asset ID": ["84444"] * 7 + ["ABC123"] * 2,
+        "Mixed dates": ["2023-07-21 00:00:00"] * 6 + ["July 23rd"] * 3,
     }
 
 
@@ -89,13 +90,29 @@ def test_sheet_with_mixed_dtypes_and_sample_rows(expected_data: dict[str, list[A
         None,
     ]
     expected_data["Asset ID"] = [84444.0] * 7 + [None] * 2
+    expected_data["Mixed dates"] = [datetime(2023, 7, 21)] * 6 + [None] * 3
 
     pd_df = sheet.to_pandas()
-    pd_assert_frame_equal(pd_df, pd.DataFrame(expected_data).astype({"Date": "datetime64[ms]"}))
+    pd_assert_frame_equal(
+        pd_df,
+        pd.DataFrame(expected_data).astype(
+            {
+                "Date": "datetime64[ms]",
+                "Mixed dates": "datetime64[ms]",
+            }
+        ),
+    )
 
     pl_df = sheet.to_polars()
     pl_assert_frame_equal(
-        pl_df, pl.DataFrame(expected_data, schema_overrides={"Date": pl.Datetime(time_unit="ms")})
+        pl_df,
+        pl.DataFrame(
+            expected_data,
+            schema_overrides={
+                "Date": pl.Datetime(time_unit="ms"),
+                "Mixed dates": pl.Datetime(time_unit="ms"),
+            },
+        ),
     )