More Converting operations #133
Conversation
core/src/main/kotlin/org/jetbrains/kotlinx/dataframe/impl/api/parse.kt
Outdated
Show resolved
Hide resolved
|
Hi! Thank you for the PR. Parsing numbers is quite an old problem here :) #9 |
|
Review current version, please. Also do you think we should make rendering test locale-independent as was suggested here? |
| Locale.setDefault(Locale.forLanguageTag("ru-RU")) | ||
|
|
||
| columnDot.convertTo<Double>().shouldBe(columnOf(12.345, 67.89)) | ||
| columnComma.convertTo<Double>().shouldBe(columnOf(12345.0, 67890.0)) |
There was a problem hiding this comment.
Shouldn't it be 12.345, 67.890? Looks suspicious that default convertTo works identically for en_US and ru_RU.
There was a problem hiding this comment.
Fixed, thanks! But I have not found a better way than calling DataColumn<String>.convertToDouble directly from universal convertTo<>.
| val options = if (locale != null) ParserOptions( | ||
| locale = locale | ||
| ) else null | ||
| return parserToDoubleWithOptions.toConverter(options) |
There was a problem hiding this comment.
You can use parserToDoubleWithOptions.applyOptions here, i think
There was a problem hiding this comment.
Done. Originally I used toConverter for getting TypeConversionException to be thrown. Now it is thrown in .convertToDouble function and this is renamed to getDoubleParser.
| */ | ||
| @JvmName("convertToDoubleFromStringNullable") | ||
| public fun DataColumn<String?>.convertToDouble(locale: Locale? = null): DataColumn<Double?> { | ||
| if (locale is Locale) { |
There was a problem hiding this comment.
What do you think?
val converter = if (locale != null) {
Parsers.getDoubleConverter(locale)
} else {
Parsers.getDoubleConverter()
}There was a problem hiding this comment.
And then simply call map { converter(it) }? This doesn't seem to work because we wanted the scalar converter not to use fallback logic. And it is implemented here now…
Sure. I guess right now it's impossible to run all tests on a PC with locale that uses comma? Would be nice to fix it |
Until now I used
Done. |
When working with specific local settings, we still can get data in international format. And it should be parsed rather than rejected. Here we have fallback on parsing doubles with POSIX number format. (This was made for
convertTofunction but also works on primary parsing. For example,readCsvWithFloatstest now passes on my computer without switching to empty locale manually.) May be, we should have similar logic in date, time and some other parsers.Also some new converters (to Boolean and LocalTime) have been added.