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

Improve grouping with groupRows. closes #294 #297

Merged
merged 4 commits into from
Jan 3, 2022

Conversation

JanMarvin
Copy link
Collaborator

allow different grouping options for groupRows(). Needs testing due possible breaking changes to write_file_2.cpp.

@JanMarvin JanMarvin linked an issue Nov 19, 2021 that may be closed by this pull request
@JanMarvin JanMarvin mentioned this pull request Nov 19, 2021
@JanMarvin
Copy link
Collaborator Author

If this passes the checks (did not check prio to pushing) this needs some testing. I have made changes to the write function and got mad and did not try to fully understand the old code. Also I was not entirely sure how to handle grouping, because I do not really use it and therefore might have implemented changes that are unwanted/unneeded. Checked basic functionality with LO and MS 365.

@codecov-commenter
Copy link

Codecov Report

Merging #297 (50eac9f) into master (1c0295d) will increase coverage by 0.01%.
The diff coverage is 50.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #297      +/-   ##
==========================================
+ Coverage   66.48%   66.49%   +0.01%     
==========================================
  Files          34       34              
  Lines        8939     8942       +3     
==========================================
+ Hits         5943     5946       +3     
  Misses       2996     2996              
Impacted Files Coverage Δ
src/write_file_2.cpp 0.00% <0.00%> (ø)
R/wrappers.R 52.24% <100.00%> (+0.09%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 08de207...50eac9f. Read the comment docs.

@bfreed3
Copy link

bfreed3 commented Nov 19, 2021

I figured out what I needed to with the old groupRows function. For the iris df, to group on the first instance of each Species in the xlsx file, the following code works for what I need.

groupRows(wb, sheet=1, c(2:51,53:101,103:151))

One just needs to play around with it a bit, and understand the shifting of rows going from the df to xlsx, and how Excel groups things on its side.

Thanks, Burgess

@JanMarvin
Copy link
Collaborator Author

Thanks for the follow up. I kinda figured the same when creating this pull request. You can't have different grouped rows in the same outline that are attached to each other. Excel will automatically combine both. To avoid this, you need to skip at least one row. Though this pull request allows having multiple nested groups and it fixes the previous warning message.
If you want, you could add a pull request to further improve the documentation. Which is quite short for this function and lead to the issue.

@JanMarvin
Copy link
Collaborator Author

JanMarvin commented Nov 22, 2021

darn, I got lost with open pull requests (293 != 294) anyone got a copy of this branch lying around? 😄
Edit: fixed and restored. git reflog ftw!

@JanMarvin JanMarvin added this to the merge after next release milestone Dec 6, 2021
@JanMarvin JanMarvin changed the base branch from master to development January 3, 2022 21:45
@JanMarvin JanMarvin merged commit 2b1b09b into ycphs:development Jan 3, 2022
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.

Multiple groupRows?
3 participants