Excel add new sheet without overwriting the file #157

LeandroC89 · 2022-08-28T01:30:59Z

Hello,
I've made a couple changes in order to be able to add sheets to an already existing file (without completely replacing it).

Overview

By using a keepFile parameter the user can now decide to add a new sheet to an Excel file, this parameter is optional and defaults to false to keep the current behavior by default.

Implementation

keepFile

Added the new parameter as an optional setting allowing the user to add a sheet to an existing file (if set as true), leaving it blank or setting as false will keep current behavior, completely overwriting the file.

Factory

I had to update the logic by which the workbook was determined for a couple reasons:

Current setting would always create a new file, so a new flow had to be added
Keeping the factory function was generating 0b files when trying to reuse existing file, so it is now returning a WorkBook (keeping the wb type logic as is for xls and xlsx files).

Please let me know if something here needs to be updated as this was the less easy part of the change

Test

Added test to test the feasibility of creating/overwriting an Excel file with data for a sheet1, and then the same data is written on a sheet2.
- Test checks if both sheet contents are the same

.gitignore

Added new entry so the Excel file generated by the new test is not kept in version control, as it would have changes everytime the tests were rerun.

Final comments

I hope this helps, and please let me know if there is something I should update or that could have been done in a better way.
Have a good weekend!

Signed-off-by: LeandroC89 <leandroadao3@gmail.com>

- Updated WorkBook factory as it was previously incompatible with reusing existing workbook - Revised test use generated random data Signed-off-by: LeandroC89 <leandroadao3@gmail.com>

koperagen · 2022-08-29T12:45:53Z

Hi! Thank you for the PR! What do you think about creating Workbook in the user code?
Like

val wb = ...
df1.writeExcel(wb, "Sheet1")
df2.writeExcel(wb, "Sheet2")

I like your idea better, because this way people won't have to deal with factories, streams. But still interesting to understand alternatives

LeandroC89 · 2022-08-29T16:42:44Z

Hi,
not a bad a idea, having null create and if a wb is provided it would add. I could even add that having writeExcel return the wb would help in this situation as I see most people would use this to create a new file and then add sheets to it, so this would be an easier way to get the wb.

However, this would require people to create the wb on their own and import the dependencies on their own (and apply the xls/xlsx logic that determines HSSF/XSSF). I feel this latest bit justifies leaving this to the function instead. Specially in Notebooks where the project can be imported with

%use dataframe

I'm not entirely sure as I haven't touched notebooks in a while but I believe in this situation the dependency to the workbook would also have to be added with the import statement which is not as straightforward.

Considering the pros and cons I would have this handled by the function writeExcel itself, although it can also be overloaded to allow both behaviors. (I can also submit this bit if needed)

koperagen · 2022-09-07T19:12:22Z

I agree, and writing several sheets is a common enough task to be solved by writeExcel itself. So keepFile is good. Could you also add a paragraph with an example here https://github.com/Kotlin/dataframe/blob/master/docs/StardustDocs/topics/write.md?
Create a new function in
org.jetbrains.kotlinx.dataframe.samples.api.Write and follow the instruction https://github.com/Kotlin/dataframe/blob/master/docs/contributions.md.

koperagen · 2022-09-07T19:19:24Z

dataframe-excel/src/main/kotlin/org/jetbrains/kotlinx/dataframe/io/xlsx.kt

@@ -246,9 +246,9 @@ public fun <T> DataFrame<T>.writeExcel(
    columnsSelector: ColumnsSelector<T, *> = { all() },
    sheetName: String? = null,
    writeHeader: Boolean = true,
-    factory: () -> Workbook
+    factory: Workbook


Let's remove this overload. I think i wanted to create an alias for () -> Workbook so that people could call this function like df.writeExcel(os, ..., WorkbookFactory.XLSX), and WorkbookFactory.XLSX here is a class owned by the dataframe. But i shelved the idea, so this overload adds no value compared to the one below

Looking back at it, should it be renamed to workBook now? Since it is what is really getting passed after this change. I can change it if so.

…Write for keepFile functionality - Added example of keepFile use in writeExcel to write.md Signed-off-by: LeandroC89 <leandroadao3@gmail.com>

Signed-off-by: LeandroC89 <leandroadao3@gmail.com>

koperagen · 2022-09-16T13:40:34Z

dataframe-excel/src/test/kotlin/org/jetbrains/kotlinx/dataframe/io/XlsxTest.kt

+    fun `write to new sheet when keepFile is true`() {
+        val names = (1..5).map { "column$it" }
+        val df = dataFrameOf(names).randomDouble(7)
+        val fileLoc = testResource("generated_wb.xlsx").toURI().toString().removeRange(0, 6)


This test is failing with NPE =( Let's use
Files.createTempFile("excel", ".xlsx").toFile()

Updated, thanks for the tip!

Signed-off-by: LeandroC89 <leandroadao3@gmail.com>

Jolanrensen · 2022-11-28T13:24:28Z

@LeandroC89 Could you update the pr to the newest master? Then we might be able to merge it for the 0.9.0 release

# Conflicts: # dataframe-excel/src/main/kotlin/org/jetbrains/kotlinx/dataframe/io/xlsx.kt # dataframe-excel/src/test/kotlin/org/jetbrains/kotlinx/dataframe/io/XlsxTest.kt # docs/StardustDocs/topics/write.md # tests/src/test/kotlin/org/jetbrains/kotlinx/dataframe/samples/api/Write.kt

LeandroC89 · 2023-12-17T21:29:48Z

Sorry it took so long. At the time there was an issue with Korro, and I haven't checked this one in a while after the import was removed! It touches a few more files now due to having taken all the changes since originally PR'ed.

koperagen · 2023-12-19T11:25:07Z

Run formatKotlin task please, then i can merge it

LeandroC89 added 3 commits August 27, 2022 11:39

- Added test for new sheet implementation

10f5c4f

Signed-off-by: LeandroC89 <leandroadao3@gmail.com>

- Removed unnecessary logging from new sheet test

27c6396

Signed-off-by: LeandroC89 <leandroadao3@gmail.com>

- Added addSheet functionality (keepFile parameter)

a5f8a1b

- Updated WorkBook factory as it was previously incompatible with reusing existing workbook - Revised test use generated random data Signed-off-by: LeandroC89 <leandroadao3@gmail.com>

koperagen reviewed Sep 7, 2022

View reviewed changes

LeandroC89 added 2 commits September 12, 2022 01:10

- Added new function in org.jetbrains.kotlinx.dataframe.samples.api.…

679d7ff

…Write for keepFile functionality - Added example of keepFile use in writeExcel to write.md Signed-off-by: LeandroC89 <leandroadao3@gmail.com>

- Fixed variable use in added function for documentation

9182ad6

Signed-off-by: LeandroC89 <leandroadao3@gmail.com>

koperagen reviewed Sep 16, 2022

View reviewed changes

- Changed writeExcel location to temporary file

b45557a

Signed-off-by: LeandroC89 <leandroadao3@gmail.com>

Jolanrensen added this to the 0.9.0 milestone Nov 28, 2022

Jolanrensen added the enhancement New feature or request label Nov 28, 2022

Jolanrensen removed this from the 0.9.0 milestone Dec 15, 2022

koperagen merged commit a33e104 into Kotlin:master Jan 10, 2024
1 check was pending

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Excel add new sheet without overwriting the file #157

Excel add new sheet without overwriting the file #157

LeandroC89 commented Aug 28, 2022 •

edited

koperagen commented Aug 29, 2022

LeandroC89 commented Aug 29, 2022 •

edited

koperagen commented Sep 7, 2022

koperagen Sep 7, 2022

LeandroC89 Sep 12, 2022

koperagen Sep 16, 2022

LeandroC89 Oct 23, 2022

Jolanrensen commented Nov 28, 2022

LeandroC89 commented Dec 17, 2023

koperagen commented Dec 19, 2023

Excel add new sheet without overwriting the file #157

Excel add new sheet without overwriting the file #157

Conversation

LeandroC89 commented Aug 28, 2022 • edited

Overview

Implementation

keepFile

Factory

Test

.gitignore

Final comments

koperagen commented Aug 29, 2022

LeandroC89 commented Aug 29, 2022 • edited

koperagen commented Sep 7, 2022

koperagen Sep 7, 2022

Choose a reason for hiding this comment

LeandroC89 Sep 12, 2022

Choose a reason for hiding this comment

koperagen Sep 16, 2022

Choose a reason for hiding this comment

LeandroC89 Oct 23, 2022

Choose a reason for hiding this comment

Jolanrensen commented Nov 28, 2022

LeandroC89 commented Dec 17, 2023

koperagen commented Dec 19, 2023

LeandroC89 commented Aug 28, 2022 •

edited

LeandroC89 commented Aug 29, 2022 •

edited