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

Add consts for Excel max sheet size and @assert them in writetable! #247

Merged
merged 1 commit into from
Dec 7, 2023

Conversation

nathanrboyer
Copy link
Contributor

This fixes #246.

I found these limits hardcoded in other locations besides write.jl, so I replaced them with the const variables everywhere they appear.

MWE:

julia> using XLSX, DataFrames

julia> file = normpath(homedir(), "Downloads/test.xlsx");

julia> df = DataFrame(Symbol.(1:16_385) .=> "data")
1×16385 DataFrame
 Row │ 1       2       3       4       5       6       7       8       9       10      11  
     │ String  String  String  String  String  String  String  String  String  String  Str 
─────┼──────────────────────────────────────────────────────────────────────────────────────
   1 │ data    data    data    data    data    data    data    data    data    data    dat 

julia> df2 = DataFrame(x=1:1_048_576)
1048576×1 DataFrame
     Row │ x       
         │ Int64
─────────┼─────────
       11
       22
       33
    

julia> XLSX.writetable(file, df, overwrite=true)
ERROR: AssertionError: `data` contains 16385 columns, but Excel only supports up to 16384; must reduce `data` size

julia> XLSX.writetable(file, df2, overwrite=true)
ERROR: AssertionError: `data` contains 1048576 rows, but Excel only supports up to 1048575; must reduce `data` size

Note that df2 needs to fail at row_count==1_048_576 rows because the column header puts it over the limit.

I would need help formalizing this MWE into tests if that is required.

Copy link

codecov bot commented Dec 5, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (063b2e2) 94.76% compared to head (6d6df49) 94.76%.

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #247   +/-   ##
=======================================
  Coverage   94.76%   94.76%           
=======================================
  Files          15       15           
  Lines        2005     2007    +2     
=======================================
+ Hits         1900     1902    +2     
  Misses        105      105           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@felipenoris
Copy link
Owner

looks good. thanks!

@felipenoris felipenoris merged commit 8e63de8 into felipenoris:master Dec 7, 2023
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

writetable bug for large DataFrame
2 participants