Skip to content

Conversation

@jmrsnt
Copy link
Contributor

@jmrsnt jmrsnt commented Oct 24, 2023

This PR is to fix the issue #483

@jmrsnt jmrsnt changed the title Fix reading Excel file that has cells with formulas that returns strings Fix reading Excel file that has cells with formulas that return string Oct 24, 2023
@Jolanrensen Jolanrensen self-assigned this Oct 25, 2023
@Jolanrensen Jolanrensen added this to the 0.13.0 milestone Oct 25, 2023
@Jolanrensen Jolanrensen added the bug Something isn't working label Oct 25, 2023
@Jolanrensen
Copy link
Collaborator

Thanks for the fix! Might I just request a recursive call instead of listing all cellType->valueTypes twice? Might also make it easier to extend in the future if we get more types. Something like:

private fun Cell?.cellValue(sheetName: String): Any? { if (this == null) return null fun getValueFromType(type: CellType?): Any? = when (type) { CellType._NONE -> error("Cell $address of sheet $sheetName has a CellType that should only be used internally. This is a bug, please report https://github.com/Kotlin/dataframe/issues") CellType.NUMERIC -> { val number = numericCellValue when { DateUtil.isCellDateFormatted(this) -> DateUtil.getLocalDateTime(number).toKotlinLocalDateTime() else -> number } } CellType.STRING -> stringCellValue CellType.FORMULA -> getValueFromType(cachedFormulaResultType) CellType.BLANK -> stringCellValue CellType.BOOLEAN -> booleanCellValue CellType.ERROR -> errorCellValue null -> null } return getValueFromType(cellType) }

(and renaming "greather" to "greater" in the test :) )

@Jolanrensen Jolanrensen self-requested a review October 25, 2023 11:08
@Jolanrensen Jolanrensen removed their assignment Oct 25, 2023
@Jolanrensen Jolanrensen removed this from the 0.13.0 milestone Oct 25, 2023
@jmrsnt
Copy link
Contributor Author

jmrsnt commented Oct 25, 2023

Yes, it makes sense. Thank you for observing, I am sending the requested changes.

@Jolanrensen Jolanrensen added this to the 0.13.0 milestone Oct 26, 2023
@Jolanrensen Jolanrensen merged commit 10fc58c into Kotlin:master Oct 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

2 participants