add nulls parse option in readExcel #1247
Conversation
| @@ -255,6 +271,8 @@ public fun DataFrame.Companion.readExcel( | |||
| * when set to false, it operates as [NameRepairStrategy.MAKE_UNIQUE], | |||
| * ensuring unique column names will make the columns be named according to excel columns, like "A", "B", "C" etc. | |||
| * for unstructured data. | |||
| * @param parseEmptyAsNull when set to true, empty strings in cells are parsed as null (default true). | |||
There was a problem hiding this comment.
There may be more representations in strings that can refer to null. maybe it might be better to follow readCsv and other parsing options for this. It will be overkill to add the full ParserOptions as argument (since datetime and locales are already handled by excel), but csv, for instance uses nullStrings: Set<String>= setOf("", "NA", "N/A", "null", "NULL", "None", "none", "NIL", "nil"), so it might make sense to add a nullStrings argument here as well, probably with fewer defaults
There was a problem hiding this comment.
Created an issue:
#1248
There was a problem hiding this comment.
I don't think is a good idea to introduce a new non-trivial logic just before release, need more time for implementation and testing.
There was a problem hiding this comment.
okay sure, makes sense
There was a problem hiding this comment.
speaking off, this is a binary-breaking change. Since we released a beta already, we cannot break the API like that without a fallback mechanism. Could you add the old function signatures back with @Deprecated() at level HIDDEN? That way, we won't break libraries calling the old function :)
Fixes #1166.