Adding write support to `File_Format.Excel` #3551

jdunkerley · 2022-06-28T11:26:45Z

Pull Request Description

Support for writing tables to Excel.

Important Notes

Has custom support for Error mode as will allow appending a new table in this mode to the file.

Checklist

Please include the following checklist in your PR:

The documentation has been updated if necessary.
All code conforms to the
Scala,
Java,
and
Rust
style guides.
All code has been tested:
- Unit tests have been written where possible.

std-bits/table/src/main/java/org/enso/table/write/ExcelWriter.java

JaroslavTulach

Thanks for the example with date in CSV files.

JaroslavTulach · 2022-07-01T14:46:15Z

test/Table_Tests/src/Excel_Spec.enso

+           for these tests. This should ideally be re-enabled with the
+           completion of the following story:
+           https://www.pivotaltracker.com/story/show/181755990
+        no_dates = File_Format.Delimited "," value_formatter=(Data_Formatter date_formats=[] time_formats=[] datetime_formats=[])


I see. How would one enable parsing of the dates again? Probably by avoiding value_formatter specification...

exactly - if value_formatter is not set it should automatically parse date in the format of yyyy-MM-dd

Alternatively:
no_dates = File_Format.Delimited "," value_formatter=(Data_Formatter date_formats=['yyyy-MM-dd'] time_formats=[] datetime_formats=[])
would just re-enable dates (leaving DateTimes and Times as text).

How shall I run the test? I tried:

enso$ ./built-distribution/enso-engine-0.0.0-dev-linux-amd64/enso-0.0.0-dev/bin/enso --run test/Table_Tests/src/Excel_Spec.enso In module Excel_Spec: Compiler encountered errors: Excel_Spec.enso[18:1-18:19]: The `project` keyword was used in an import statement, but the module does not belong to a project. Execution finished with an error: Compilation aborted due to errors.

I deleted the 18th line, but then most of the tests fail.

Need the --in-project argument:

Windows:
.\bin\enso --ir-caches --run C:\Repos\Enso\ide\test\Table_Tests\src\Excel_Spec.enso --in-project C:\Repos\Enso\ide\test\Table_Tests

distribution/lib/Standard/Base/0.0.0-dev/src/System/File.enso

distribution/lib/Standard/Base/0.0.0-dev/src/System/File/Existing_File_Behavior.enso

distribution/lib/Standard/Table/0.0.0-dev/src/Io/Excel.enso

distribution/lib/Standard/Table/0.0.0-dev/src/Io/File_Format.enso

std-bits/table/src/main/java/org/enso/table/excel/ExcelRange.java

std-bits/table/src/main/java/org/enso/table/excel/ExcelRow.java

std-bits/table/src/main/java/org/enso/table/write/ExcelWriter.java

test/Table_Tests/src/Excel_Spec.enso

radeusgd · 2022-07-04T13:14:36Z

test/Table_Tests/src/Excel_Spec.enso

+        Test.specify 'should result in Range_Exceeded error if trying to write in too small a range' <|
+            out.delete_if_exists
+            (Enso_Project.data / test_sheet_name) . copy_to out
+            sub_clothes.write out (File_Format.Excel (Cell_Range "Another!A1:B2")) . should_fail_with Range_Exceeded


Is the target file modified? I'd like the test to check that.

We could do with a File get_modified_time but as lacking that will hold off adding for now. Will add as part of append.

radeusgd

Looks good to me overall, some comments and questions (possibly for future) in-line.

The major thing that I think should be addressed before merging is that documentation of Existing_File_Behavior.Error is not consistent with the behavior for Excel files. While I understand the rationale behind the changed behavior, I don't think we can have documentation that is not aligned with the actual behavior as that will greatly confuse the users, so I think we need to figure something out for this case.

- Added File.copy_to - Removed writeSpreadsheetCell. - Added ExcelWriter.

Still to do Range based.

Header Infer based on current

Fix issues with BLANK cells.

…ing_File_Behavior.enso Co-authored-by: Radosław Waśko <radoslaw.wasko@enso.org>

Co-authored-by: Radosław Waśko <radoslaw.wasko@enso.org>

std-bits/table/src/main/java/org/enso/table/write/ExcelWriter.java

…java Co-authored-by: Radosław Waśko <radoslaw.wasko@enso.org>

jdunkerley requested a review from hubertp June 28, 2022 14:20

jdunkerley force-pushed the wip/jd/excel-write-182309816 branch from 3b5dca5 to 51a19d7 Compare June 30, 2022 16:51

jdunkerley marked this pull request as ready for review June 30, 2022 16:54

jdunkerley requested review from 4e6, radeusgd and JaroslavTulach as code owners June 30, 2022 16:54

hubertp approved these changes Jul 1, 2022

View reviewed changes

std-bits/table/src/main/java/org/enso/table/write/ExcelWriter.java Outdated Show resolved Hide resolved

jdunkerley force-pushed the wip/jd/excel-write-182309816 branch from b85ad70 to 37d6a67 Compare July 1, 2022 13:08

JaroslavTulach approved these changes Jul 1, 2022

View reviewed changes

radeusgd reviewed Jul 4, 2022

View reviewed changes

radeusgd requested changes Jul 4, 2022

View reviewed changes

jdunkerley requested a review from radeusgd July 4, 2022 15:18

jdunkerley and others added 17 commits July 4, 2022 16:27

Initial work for Excel write.

68fb00c

- Added File.copy_to - Removed writeSpreadsheetCell. - Added ExcelWriter.

Close to working for a Sheet write.

5fa4424

Still to do Range based.

Remove old Writer.java

5a91651

Excel Write work.

80d4edf

Excel Write work.

beef221

Fix failing tests.

3e15d30

Move the expand range to ExcelRange.

8d4bc60

Expand the write data to support range based writing.

64ef283

Range writing

6178d65

Header Infer based on current

Handle errors.

2ba19af

Some more tests.

300b1f2

Fix issues with BLANK cells.

Lots of tests...

13faa7e

Java Format.

a6fc68c

ChangeLog

67e8ba3

Workaround for an issue with POI xls mode.

413819d

Remove old test file.

f208421

Update distribution/lib/Standard/Base/0.0.0-dev/src/System/File/Exist…

4f601b9

…ing_File_Behavior.enso Co-authored-by: Radosław Waśko <radoslaw.wasko@enso.org>

jdunkerley and others added 7 commits July 4, 2022 16:27

Update distribution/lib/Standard/Base/0.0.0-dev/src/System/File.enso

a51a2a4

Co-authored-by: Radosław Waśko <radoslaw.wasko@enso.org>

Update distribution/lib/Standard/Table/0.0.0-dev/src/Io/Excel.enso

b6ad551

Co-authored-by: Radosław Waśko <radoslaw.wasko@enso.org>

Update distribution/lib/Standard/Table/0.0.0-dev/src/Io/Excel.enso

695bfa1

Co-authored-by: Radosław Waśko <radoslaw.wasko@enso.org>

Most PR comments.

f06e174

More PR comments.

1663a1e

Missing comment.

6de119e

Rename method.

5e0a68e

radeusgd approved these changes Jul 4, 2022

View reviewed changes

std-bits/table/src/main/java/org/enso/table/write/ExcelWriter.java Outdated Show resolved Hide resolved

jdunkerley force-pushed the wip/jd/excel-write-182309816 branch from 4733e64 to 5e0a68e Compare July 4, 2022 15:58

Update std-bits/table/src/main/java/org/enso/table/write/ExcelWriter.…

715ef6e

…java Co-authored-by: Radosław Waśko <radoslaw.wasko@enso.org>

jdunkerley added the CI: Ready to merge This PR is eligible for automatic merge label Jul 4, 2022

Java format.

1011ca9

mergify bot merged commit 4ca2097 into develop Jul 4, 2022

mergify bot deleted the wip/jd/excel-write-182309816 branch July 4, 2022 18:32

wdanilo mentioned this pull request Feb 6, 2023

Review and improve In-Memory Table support for Dates #4459

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding write support to `File_Format.Excel` #3551

Adding write support to `File_Format.Excel` #3551

jdunkerley commented Jun 28, 2022 •

edited

Loading

JaroslavTulach left a comment

JaroslavTulach Jul 1, 2022

jdunkerley Jul 1, 2022

JaroslavTulach Jul 1, 2022

jdunkerley Jul 1, 2022

radeusgd Jul 4, 2022

jdunkerley Jul 4, 2022

radeusgd left a comment

Adding write support to File_Format.Excel #3551

Adding write support to File_Format.Excel #3551

Conversation

jdunkerley commented Jun 28, 2022 • edited Loading

Pull Request Description

Important Notes

Checklist

JaroslavTulach left a comment

Choose a reason for hiding this comment

JaroslavTulach Jul 1, 2022

Choose a reason for hiding this comment

jdunkerley Jul 1, 2022

Choose a reason for hiding this comment

JaroslavTulach Jul 1, 2022

Choose a reason for hiding this comment

jdunkerley Jul 1, 2022

Choose a reason for hiding this comment

radeusgd Jul 4, 2022

Choose a reason for hiding this comment

jdunkerley Jul 4, 2022

Choose a reason for hiding this comment

radeusgd left a comment

Choose a reason for hiding this comment

Adding write support to `File_Format.Excel` #3551

Adding write support to `File_Format.Excel` #3551

jdunkerley commented Jun 28, 2022 •

edited

Loading