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

Combining merged cells producing incorrect result when used on a non-default cell range #1330

Closed
pcasteran opened this issue Oct 23, 2023 · 1 comment · Fixed by #1335
Closed
Assignees
Milestone

Comments

@pcasteran
Copy link

pcasteran commented Oct 23, 2023

Describe the bug
Calling Worksheet.get_values() with combine_merged_cells=True and a range name not starting at column A or row 1 ("B:C" for example) produces incorrect results when expanding the merged cells content.

To Reproduce
Create a worksheet with cells B1 and B2 merged and some other content on C1 and C2.
For example:

A B C
foo dummy 1
dummy 2

A call to ws.get_values("B:C", combine_merged_cells=True) outputs:

  • ["foo", "dummy 1"]
  • ["", "dummy 1"] => no "foo" and "dummy 1" instead of "dummy 2"

Expected behavior
The content of B1 is expanded and available on both rows:

  • ["foo", "dummy 1"]
  • ["foo", "dummy 2"]

Environment info:

  • Operating System: Linux,
  • Python version: 3.11
  • gspread version: 5.11.3

Additional context
The issue is that the combined_merge_values() function does not take into account the requested cell range to offset the value expansion.

@alifeee
Copy link
Collaborator

alifeee commented Oct 24, 2023

Hi! Thanks for the report. This is a pretty glaringly wrong problem with combine_merged_cells. Adding this feature (which I did...) has caused a lot of pain, haha.

Perhaps we should have released the feature as experimental. Anyway, I have made a branch with a failing test.

If this test passes, then the bug is fixed. I will attempt the fix when I get time. If you would like to give it a go, please do.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants