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

PRs that were forgotten in development #483

Merged
merged 49 commits into from
Aug 11, 2024
Merged

PRs that were forgotten in development #483

merged 49 commits into from
Aug 11, 2024

Conversation

JanMarvin
Copy link
Collaborator

@JanMarvin JanMarvin commented Jul 26, 2024

PRs from the past.

⚠️ This needs testing! ⚠️

@JanMarvin JanMarvin changed the title Development [WIP] PRs that were forgotten in development Jul 26, 2024
JanMarvin and others added 28 commits August 11, 2024 13:59
…copy&pasted over everything until I found the expected values in my xml file.
…ich potentially needs a unique number format ID

Make sure that writeData with a date creates a style that has no collisions of its numFmtId with existing styles. By default, a style with numFmdId=165 (hard-coded!) is created by createStyle. When inserting that style into a workbook, we need to assign it a new, unique numFmtId if the workbook already has some custom number formats.
The explicitly-called addStyle function checked for this case, but the implicitly-called classStyles function did not.

Fixes #330.
…ich potentially needs a unique number format ID

Make sure that writeData with a date creates a style that has no collisions of its numFmtId with existing styles. By default, a style with numFmdId=165 (hard-coded!) is created by createStyle. When inserting that style into a workbook, we need to assign it a new, unique numFmtId if the workbook already has some custom number formats.
The explicitly-called addStyle function checked for this case, but the implicitly-called classStyles function did not.

Fixes #330.
christianrickert and others added 19 commits August 11, 2024 14:11
The function call to tempfile places the location of tmpDir at the per-session temporary directory, see Details section: https://www.rdocumentation.org/packages/base/versions/3.6.2/topics/tempfile
There is IMHO no need to recursively create the path to tmpDir: all path components will be available when the last
component in the tmpDir path has been created. In fact, recursive directory creation can create problems on Windows,
when the TEMP / TMP variable has been set to the root directory of an external drive ("D:\").
My suggestion is to use the  dir.exists function to check if a file object exists and if it is a directory. See ?dir.exists for base::dir.exists -
"dir.exists checks that the paths exist (in the same sense as file.exists) and are directories."

In the documentation for file.exists, there's system-dependend limitations mentioned, in particular "What constitutes a ‘file’ is system-dependent, but should include directories. (However, directory names must not include a trailing backslash or slash on Windows.)":
https://www.rdocumentation.org/packages/base/versions/3.6.2/topics/files
-) Preserve the hidden attribute for all grouped cells. If no hidden attribute was given, preserve NA. Otherwise either saving breaks (issue #138) or the hidden rows are shifted.

Fixes #138
-) Preserve grouping level
-) Fix crashes
-) Improve nested columns (also manually created by openxlsx)
-) Properly set max outline level for rows/cols in the xml
…o arbitrary nesting

-) groupRows did not properly overwrite existing groupings / append new ones
-) Add new parameter level = -1 to the groupRows funtion to let openxlsx auto-detect the next grouping level
-) Add test cast with several nested and unnested groupings to cover most use-cases
-) Saving to an .xlsx file dropped the grouping if a row didn't contain content any
-) Simplified xml-generation for grouped rows and row height.

Fixes part of #138, implements the counterpart of #105 for rows, and properly implements the remaining missing parts of #294.
ungrouping a nested grouping will decrease the grouping level for the corresponding row/column by 1 and show the row/column. If the row/column is not grouped any more, it is removed form the outlineLevel structure.
-) implement generalized version of groupColumns, including auto-detecting the outline level
-) consistent variable naming in groupRows and groupColumns
In the past, new columns were always appended to the end of the vector of <col... XML elements. This meant that the columns were not always in the correct order, so we need to sort them prior to writing to the xlsx file.
@codecov-commenter
Copy link

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

Attention: Patch coverage is 84.68208% with 53 lines in your changes missing coverage. Please review.

Project coverage is 70.71%. Comparing base (b776fdc) to head (540f669).
Report is 43 commits behind head on master.

Files Patch % Lines
src/write_data.cpp 65.45% 19 Missing ⚠️
R/helperFunctions.R 70.90% 16 Missing ⚠️
R/wrappers.R 86.20% 16 Missing ⚠️
R/openxlsxCoerce.R 0.00% 1 Missing ⚠️
src/write_file_2.cpp 95.00% 1 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #483      +/-   ##
==========================================
+ Coverage   67.98%   70.71%   +2.72%     
==========================================
  Files          34       35       +1     
  Lines        9007     9150     +143     
==========================================
+ Hits         6123     6470     +347     
+ Misses       2884     2680     -204     

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

@JanMarvin JanMarvin changed the title [WIP] PRs that were forgotten in development PRs that were forgotten in development Aug 11, 2024
@JanMarvin JanMarvin merged commit 4dd2774 into master Aug 11, 2024
18 of 19 checks passed
@JanMarvin JanMarvin deleted the development branch August 11, 2024 13:03
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.

9 participants