Worksheet ID Type Inconsistencies

[FlantasticDan]

Describe the bug
On a Spreadsheet instance when operating on the underlying worksheet objects sometimes a the worksheet id can be a string (del_worksheet_by_id), other times it must be an int (get_worksheet_by_id).

Expected behavior
Ability to get a worksheet off a spreadsheet with any valid integer or integer-like string. I frequently store Worksheet indexes in environment variables so if I forget to wrap them in int() I get a needless error.

Code example

sht = client.open('My fancy spreadsheet')
worksheet = sht.get_worksheet(1234) # this works

worksheet = sht.get_worksheet("1234") # throws gspread.exceptions.WorksheetNotFound

Environment info:

• Operating System: macOS
• Python: 3.11.4
• gspread: 5.10.0

Proposed Solution

gspread/gspread/spreadsheet.py

Line 271 in 739b8fc

def get_worksheet_by_id(self, id):

def get_worksheet_by_id(self, id):
        # ...
        sheet_data = self.fetch_sheet_metadata()

        try:
            item = finditem(
                lambda x: x["properties"]["sheetId"] == int(id), # wrap id argument with int converter
                sheet_data["sheets"],
            )
            return Worksheet(self, item["properties"])
        except (StopIteration, KeyError, ValueError): # add Value Error in case int conversion fails
            raise WorksheetNotFound("id {} not found".format(id))

It might be clearer to break out the ValueError into it's own exception handler but I don't know what the ideal error to raise would be. Happy to open a PR for this, just looking for some guidance first.

[alifeee]

Hi, this is a good catch. Looks like your gspread.exceptions.WorksheetNotFound could have been hard to find the source. You have done well.

It is a little confusing, since the spreadsheet id is a str, but the worksheet id is an int. However, I think ints should remain, so get_worksheet_by_id(2742) should be usable.

However, as you say, it is also nice to be able to use strings. In reality, I think the Google API will accept strings, so we could just always use strings (and turn int to str with str() or just leave it as an int - it should work as either).

@lavigne958 is currently in the process of typing the project, which will make things more obvious.

However, if you would like to make a PR, that would be great. See the contributing guide for help, but we will also help you if you encounter any issues :)

My suggestion would be:

• for get_worksheet_by_id
  • type the argument as int | str
  • cast id to str with str(id)
• for del_worksheet_by_id
  • type the argument as int | str
  • cast id to str with str(id)
• alter test_get_worksheet_by_id to also attempt opening with string
  

  gspread/tests/spreadsheet_test.py

  Lines 39 to 42 in 739b8fc

  	@pytest.mark.vcr()
  	def test_get_worksheet_by_id(self):
  	sheet1 = self.spreadsheet.get_worksheet_by_id(0)
  	self.assertTrue(isinstance(sheet1, gspread.Worksheet))

• (optional) add test_del_worksheet_by_id (it doesn't exist)

Thanks for the issue and let us know if you want to make a PR or any help! ♥