gspread icon indicating copy to clipboard operation
gspread copied to clipboard

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

Open saransappa opened this issue 1 year ago • 4 comments

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.

saransappa avatar Dec 11 '24 13:12 saransappa

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+ https://github.com/burnash/gspread/pull/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

alifeee avatar Dec 11 '24 18:12 alifeee

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.

saransappa avatar Dec 12 '24 17:12 saransappa

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 https://github.com/burnash/gspread/pull/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 https://github.com/burnash/gspread/commit/7c5a06a01498a2d558ab6de5126d38d758ecc4d4, where the fill_gaps function's default return value was changed from [] to [[]]

That change was part of the PR https://github.com/burnash/gspread/pull/1196 which was part of the issue https://github.com/burnash/gspread/issues/966

The only reference to the change I can see is this comment by @lavigne958: https://github.com/burnash/gspread/pull/1196#discussion_r1210687160

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...

alifeee avatar Dec 13 '24 12:12 alifeee

Okay @alifeee Lets wait for @lavigne958 's response

saransappa avatar Dec 14 '24 04:12 saransappa