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 from_dims to wb_dims #960

Merged
merged 3 commits into from Mar 1, 2024
Merged

Add from_dims to wb_dims #960

merged 3 commits into from Mar 1, 2024

Conversation

olivroy
Copy link
Collaborator

@olivroy olivroy commented Feb 29, 2024

Addresses part of #958 (comment)

I made from_dims independant from from_col / from_row, but didn't really document it. Hopefully the error should be clear enough?

@JanMarvin
Copy link
Owner

Thanks for getting the ball rolling. I was thinking about something like this:

library(openxlsx2)
dims <- wb_dims(from_dims = "A1", x = mtcars)

wb_dims(from_dims = dims, x = mtcars)
#> Error in wb_dims(from_dims = dims, x = mtcars): `from_col` / `from_row` should have length 1. and be positive.

R/utils.R Outdated Show resolved Hide resolved
@JanMarvin
Copy link
Owner

My intention is to be able to be able to further simplify the placement with wb_dims(from_dims). So that if I want to write two datasets next to each other, I could simply use the dims from the first and say, write this to the top, left, bottom, right of this dims range. Picking just one dims and let the wb_dims() magic do the rest.

Does that make sense?

@JanMarvin
Copy link
Owner

JanMarvin commented Feb 29, 2024

I took the liberty to visualize my idea in code. The commit above implements the following. Hope you can forgive me, hacking into your code. Please feel free to revert this. It's not something I have given much thought. Obviously there might be a better solution I simply overlooked:

library(openxlsx2)
  
mm <- matrix(1:4, 2, 2)

dims1 <- wb_dims(from_dims = "A1", x = mm)
dims2 <- wb_dims(from_dims = dims1, x = mm, below = 2)
dims3 <- wb_dims(from_dims = dims2, x = mm, right = 2)
dims4 <- wb_dims(from_dims = dims3, x = mm, above = 2)
dims5 <- wb_dims(from_dims = dims4, x = mm, left = 2)

dims1; dims2; dims3; dims4; dims5
#> [1] "A1:B3"
#> [1] "A5:B7"
#> [1] "D5:E7"
#> [1] "D1:E3"
#> [1] "A1:B3"

wb <- wb_workbook()$add_worksheet()$
  add_fill(dims = dims1, color = wb_color(theme = 1))$
  add_fill(dims = dims2, color = wb_color(theme = 2))$
  add_fill(dims = dims3, color = wb_color(theme = 3))$
  add_fill(dims = dims4, color = wb_color(theme = 4))

if (interactive()) wb$open()
Screenshot 2024-02-29 at 18 59 41

[Edit:] Fixed it, now it goes full circle.

R/utils.R Outdated
frow <- min(from_row)
} else if (!is.null(above)) {
fcol <- min(from_col)
frow <- min(from_row) - above - nrow(args$x)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this might require a column name correction

Copy link
Collaborator Author

@olivroy olivroy Feb 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh no, row_names col_names is a bit of a nightmare. Took me a while to figure it properly without these. (had to write my code on actual paper to figure it out)

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hehe, yeah, you've created a complex monster with wb_dims(). I have implemented something, maybe have a look if you have time. I probably wont have much time to work on this the upcoming week. Was thinking about corner cases, because ... why not wb_dims(from_dims = "A2:B3", below = 4, right = 5). But we can wait until anyone asks for it. Probably going with what we have and documenting it a bit should be enough for this PR.

@olivroy
Copy link
Collaborator Author

olivroy commented Feb 29, 2024

I see what you mean. This is more complex than I thought. I can test it a bit more thoroughly once you have merged. Let's call it very experimental for now.

@JanMarvin
Copy link
Owner

Should work now:

library(openxlsx2)
dims <- wb_dims(from_dims = "B4", right = 4)
wb <- wb_workbook()$add_worksheet()$add_data(dims = dims, x = mtcars, row_names = TRUE)

wb$add_fill(dims = wb_dims(from_dims = dims, x = mtcars, row_names = TRUE, cols = "mpg"), color = wb_color("yellow"))

if (interactive()) wb$open()

@olivroy
Copy link
Collaborator Author

olivroy commented Mar 1, 2024

I guess this is good to go. I will try to test more thoroughly after

@JanMarvin JanMarvin merged commit d208470 into main Mar 1, 2024
9 checks passed
@JanMarvin JanMarvin deleted the from-dims branch March 1, 2024 12:58
@JanMarvin
Copy link
Owner

Thanks, merged

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.

None yet

2 participants