Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix reading Excel file that has cells with formulas that return string #484

Merged
merged 3 commits into from
Oct 26, 2023

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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants