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

Inconsistent wb_dims return when row vector contains 1 #1093

Closed
sTeADone opened this issue Jul 30, 2024 · 2 comments · Fixed by #1094
Closed

Inconsistent wb_dims return when row vector contains 1 #1093

sTeADone opened this issue Jul 30, 2024 · 2 comments · Fixed by #1094
Labels
bug 🐛 Something isn't working

Comments

@sTeADone
Copy link

sTeADone commented Jul 30, 2024

wb_dims gives weird dimension range when the rows argument is a vector and the vector contains 1.

r$> packageVersion("openxlsx2")
[1] '1.8.0.9000'

r$> wb_dims(rows = c(2,30), cols = 2)
[1] "B2:B2,B30:B30"

r$> wb_dims(rows = c(1,30), cols = 2)
[1] "B1:B2"

r$> wb_dims(rows = c(2,30), cols = c(2,4))
[1] "B2:B2,D2:D2,B30:B30,D30:D30"

r$> wb_dims(rows = c(1,30), cols = c(2,4))
[1] "B1:B2,D1:D2"

r$> wb_dims(rows = c(2,1), cols = c(2,4))
[1] "B1:B2,D1:D2"

r$> wb_dims(rows = c(5,1), cols = c(2,4))
[1] "B1:B2,D1:D2"
@JanMarvin
Copy link
Owner

Thanks, indeed. I probably my latest patches were not good enough :(

packageVersion("openxlsx2")
wb_dims(rows = c(2,30), cols = 2)      # expect B2,B30
wb_dims(rows = c(1,30), cols = 2)      # expect B1,B30
wb_dims(rows = c(2,30), cols = c(2,4)) # expect B2,B30,D2,D30
wb_dims(rows = c(1,30), cols = c(2,4)) # expect B1,B30,D1,D30
wb_dims(rows = c(2,1), cols = c(2,4))  # expect B2,B1,D2,D1 (or sorted B1:B2,D1:D2)
wb_dims(rows = c(5,1), cols = c(2,4))  # expect B5,B1,D5,D1 (or sorted B1,B5,D1,D5)

@JanMarvin
Copy link
Owner

I've opened a PR in #1094, which should fix the issue. We still return ranges for single cells "A1:A1", but that's a cosmetic thing. Hopefully one day in the future we will get better in reducing. Something like this:

wb_dims(rows = c(5, 1:3), cols = c(1:3, 5)) %>% 
  dims_to_dataframe(fill = TRUE) %>% 
  dataframe_to_dims(dim_break = TRUE)

This could reduce to: "A1:C3,A5:C5,E1:E3,E5" but we are not there yet.

@olivroy olivroy added the bug 🐛 Something isn't working label Aug 5, 2024
JanMarvin added a commit that referenced this issue Aug 5, 2024
… returned. fixes #1093 (#1094)

* [wb_dims] without `x` and non sequential rows, incorrect results were returned. fixes #1093

* [wb_dims] move code up so that frow creation is in a single condition

* [wb_dims] update NEWS
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something isn't working
Projects
None yet
3 participants