Support mix of date/timestamp and strings

PrettyWood commented 4 months ago

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
- 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

0: Could not build schema for sheet Sheet2
1: could not figure out column type for following type combination: {Timestamp(Millisecond, None), Utf8}

lukapeschke commented 4 months ago

IMO, we will have to take a design decision here, I see two paths:

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
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 ?

PrettyWood commented 4 months ago

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?

lukapeschke commented 4 months ago

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:

durgeksh commented 4 months ago

I was about to report this error. For below data I am also getting error as:

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

noctuid commented 4 months ago

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.

PrettyWood commented 4 months ago

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!

lukapeschke commented 4 months ago

FYI I'm working on #173 , which should allow to enforce a string dtype for a given column and convert datetime objects to strings

noctuid commented 4 months ago

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.