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

worksheet.get_all_values() output format is different between 5.3.2 and 6.1.4 for empty worksheet #1533

Open
saransappa opened this issue Dec 11, 2024 · 4 comments

Comments

@saransappa
Copy link

Description

For an empty worksheet,
With gspread 5.3.2, worksheet.get_all_values() returns []
With gspread 6.1.4, worksheet.get_all_values() returns [[]]

Steps to Reproduce

Steps to reproduce the behavior:

  1. Create an empty worksheet
  2. Read the worksheet using worksheet.get_all_values()

Expected behavior

The output should be consistent across versions i.e., worksheet.get_all_values() should return [] in both 5.3.2 and 6.1.4 versions

Environment info

  • Operating System - Linux and macOS
  • Python version - 3.9.19
  • gspread version - 6.1.4

Additional context

I am using the output of this function to determine the last used row ID.
My function checks the length of this output and returns that as a last-used row ID.
Due to this difference in output, my last used row ID got incremented by 1 after gspread upgrade.

@alifeee
Copy link
Collaborator

alifeee commented Dec 11, 2024

hi, thanks for opening this issue !

really, this is a breaking change between v5 and v6, which should be included in the migration guide -> https://github.com/burnash/gspread/pulls?q=get_values+

so, since this is a change between major versions, we oughn't change it to preserve backwards compatibility

if you want to read about the history of get, get_values, and get_all_values, please read: https://github.com/burnash/gspread/pulls?q=get_values+ #1296

I advise you to either stick with v5, or update the code to deal with the special case of [[]]. Unfortunately, the nature of representing 2D data with 1D nested arrays resulted in this return code. We also could have returned None or [], but all are a personal choice.

I will let @lavigne958 provide thoughts, too, but I do not think we should change [[]] to [] in version 6

@saransappa
Copy link
Author

saransappa commented Dec 12, 2024

Thank you for the reply @alifeee

Unfortunately, the nature of representing 2D data with 1D nested arrays resulted in this return code. We also could have returned None or [], but all are a personal choice.

I understand this is a personal choice. But, I have a slightly different opinion. The output of this function is a list of rows with data, so for an empty sheet, we should return [] because there is no data at all, right?

As you mentioned, I can handle this in my code. But I just wanted to tell you my opinion.

@alifeee
Copy link
Collaborator

alifeee commented Dec 13, 2024

thanks for the response! After consideration, I think I do prefer your opinion ([] over [[]]).

The change I will discuss here is the change of the return of get_values() on an empty sheet from [] to [[]]

Above, I said that the change was a part of #1296, but looking into it, this is false. That PR only proliferated the existing issue, which was part of the v6.0.0 branch for a while.

It was introduced in 7c5a06a, where the fill_gaps function's default return value was changed from [] to [[]]

That change was part of the PR #1196 which was part of the issue #966

The only reference to the change I can see is this comment by @lavigne958: #1196 (comment)

we should update the return value in case of value error to: [[]] please.

I'm not sure why this change was made. Perhaps @lavigne958 can comment?

However, since the change was never documented (in the documentation, nor the v5->v6 migration guide), I reckon that it is a breaking change, so one could make the case for reverting the change.

I will let @lavigne958 comment, and then we can think about what to do...

@saransappa
Copy link
Author

Okay @alifeee
Lets wait for @lavigne958 's response

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

No branches or pull requests

2 participants