gspread 6.0.0 no longer compatible with gspread-formatting

[HadasManor]

Describe the bug
gspread 6.0.0 no longer compatible with gspread-formatting. Specifically, get_conditional_format_rules(worksheet) raises an AttributeError: 'Worksheet' object has no attribute 'spreadsheet'. Did you mean: 'spreadsheet_id'?. I checked and this is because indeed, the Worksheet object no longer has the attribute spreadsheet, but only spreadsheet_id (as one might see when printing worksheet.__dict__).

To Reproduce
Steps to reproduce the behavior:

1. on gspread 6.0.0 create a spreadsheet
2. worksheet = spreadsheet.get_worksheet(0)
3. worksheet.spreadsheet will raise an error. This is important because the gspread-formatting function, get_conditional_format_rules, uses worksheet.spreadsheet.

Expected behavior
I would have expected worksheet to have the attribute spreadsheet and that will sort out the incompatibility with gspread-formatting.

Code example*

import gspread
from gspread_formatting import get_conditional_format_rules

credentials = Credentials.from_service_account_info(<token>, scopes=['https://www.googleapis.com/auth/spreadsheets',
                           'https://www.googleapis.com/auth/drive.file',
                           'https://www.googleapis.com/auth/drive',
                           'https://spreadsheets.google.com/feeds'])
client = gspread.authorize(credentials)
worksheet = client.create(title=<title>, folder_id=<folder_id>)

get_conditional_format_rules(worksheet)

Environment info:

• Operating System macOS
• Python version 3.10
• gspread version 6.0.0

[alifeee]

hi! thanks for this issue! :)

I can see that gspread-formatting does not work with v6.0.0.

For now, I recommend using the previous version of gspread with

pip install gspread==5.12.4

Then, we will try and fix this issue.

The problem was introduced by commit e212e5c at worksheet.py

class Worksheet
- def __init__(self, spreadsheet, properties):
-   self.spreadsheet = spreadsheet
-   self.client = spreadsheet.client
+ def __init__(self, spreadsheet_id, client, properties):
+   self.spreadsheet_id = spreadsheet_id
+   self.client = client

So the Worksheet.spreadsheet property was removed. This was done as part of #1190, which is not very clear as to why. Perhaps @lavigne958 can help.

It could be possible to reintroduce Worksheet.spreadsheet as a method-property, like

class Worksheet
+ @property
+ def spreadsheet(self)
+   ...

but I am not sure. A question for @lavigne958 and why the change was motivated in the first place.

[HadasManor]

Thanks @alifeee! Just to clarify- any way this issue will be resolved right? Either spreadhseet will be re-introduced, or gspread-formatting will be adjusted?

[isdabenx]

Hello everyone,

I wanted to let you know that I am also experiencing this issue with gspread 6.0.0 and gspread_dataframe. When using get_as_dataframe(worksheet, True), it triggers the error AttributeError: 'Worksheet' object has no attribute 'spreadsheet', similar to the problem reported with get_conditional_format_rules(worksheet) in gspread-formatting.

I hope this information helps others who might come across the same issue. I am looking forward to a permanent fix.

Thank you!

[lavigne958]

Hi everyone thank you for raising the issue.

I apologize for breaking the use of third-party libraries relying on gspread 😑 I don't use them and they are not part of our test suite (this is something to consider in the long time).

I removed the spreadsheet attribute from the worksheet in order to better architecture the project.

A worksheet instance only needs to know the ID of its owner spreadsheet and the HTTP Client in order to make its requests.

I am not in favor of adding back the spreadsheet attribute in the worksheet, instead I would recommend the other projects to adapt to the new architecture of gspread 6.x.y please.

I'll be happy to help too with PRs after we fix/clean gspread.

Note: having the spreadsheet attribute was introducing circular decency which triggered me to remove it.

Hope you'll understand and hope it makes sense too.

[alifeee]

Thanks @alifeee! Just to clarify- any way this issue will be resolved right? Either spreadhseet will be re-introduced, or gspread-formatting will be adjusted?

These are the two options. Personally I am a fan of the former (reintroduce spreadsheet as a computed property of worksheet). However, I know @lavigne958 likely has a good reason that it was removed, and that it cannot be reintroduced(?)

I apologize for breaking the use of third-party libraries relying on gspread 😑 I don't use them and they are not part of our test suite (this is something to consider in the long time).
I removed the spreadsheet attribute from the worksheet in order to better architecture the project.

While not in the documentation, the Worksheet.spreadsheet attribute was a public attribute. Had it have been _spreadsheet I would have less of a problem. However, it seems widely used by external libraries that we at least consider adding a workaround (via @property etc)? Do you disagree? It would be nice to not force people to pin to v5.12.4

Note: having the spreadsheet attribute was introducing circular decency which triggered me to remove it.

What was the problem with this?

[lavigne958]

Thanks @alifeee! Just to clarify- any way this issue will be resolved right? Either spreadhseet will be re-introduced, or gspread-formatting will be adjusted?

These are the two options. Personally I am a fan of the former (reintroduce spreadsheet as a computed property of worksheet). However, I know @lavigne958 likely has a good reason that it was removed, and that it cannot be reintroduced(?)

I apologize for breaking the use of third-party libraries relying on gspread 😑 I don't use them and they are not part of our test suite (this is something to consider in the long time).
I removed the spreadsheet attribute from the worksheet in order to better architecture the project.

While not in the documentation, the Worksheet.spreadsheet attribute was a public attribute. Had it have been _spreadsheet I would have less of a problem. However, it seems widely used by external libraries that we at least consider adding a workaround (via @property etc)? Do you disagree? It would be nice to not force people to pin to v5.12.4

That's true I did not take into account the visibility of the attribute which makes it publicly accessible 🤔

I still think that other libraries using gspread should adapt if we change things.

If you have access to a worksheet object, it means you have access to it's parent spreadsheet object as this is the only way to obtain a worksheet.

For now I suggest we add back the spreadsheet attribute in order to prevent breaking changes and mark it as deprecated for the next major release. We take special care to only keep the attribute and prevent any use of it.

This should allow everyone to keep working and give everyone a heads up on the coming changes.

Would this be suitable?

Note: having the spreadsheet attribute was introducing circular decency which triggered me to remove it.

What was the problem with this?

This is strongly discouraged in software design as it prevents you from changing things easily when you need to and it added extra complications for the typing too.

[HadasManor]

@lavigne958 thanks for adding back, just as a note- in my experience changes should be backwards compatible or at least be on deprecation notice for a long time, especially when it's a public attribute that other packages are using. Just to make sure- so you will mark as deprecated and if other packages will not adapt I should expect again to have issues in the future? Because in that case I will still prefer sadly to fixate my gspread version and to ignore next versions

[lavigne958]

@lavigne958 thanks for adding back, just as a note- in my experience changes should be backwards compatible or at least be on deprecation notice for a long time, especially when it's a public attribute that other packages are using. Just to make sure- so you will mark as deprecated and if other packages will not adapt I should expect again to have issues in the future? Because in that case I will still prefer sadly to fixate my gspread version and to ignore next versions

you're welcome !

I agree this is our fault to remove it without notice. 😞

So far I wish to remove that attribute yes. We need to discuss it with @alifeee in order to decide the faith of this attribute.

if we remove it, then we'll add some deprecation warning for it long before it is removed.

We are open to help everyone benefit from gspread, we can make changes if they are useful to others too and if they help you without breaking or requiring changing gspread too much out of scope.

In that regard, I have a question:

You need the attribute spreadsheet located in a Worksheet instance. This attribute provide you the parent Spreadsheet instance from the current Worksheet.
what is preventing you from using the actual Spreaddsheet instance you previously used to obtain the Worksheet instance ?
if this is can be solved by other means and allow us to remove any cyclic dependency in our code I'm gladly happy to help 🙃

[HadasManor]

Thanks for the response! From your question I see there is a bit of confusion, I am not myself using the spreadsheet attribute of Worksheet, but rather I am using gspread in conjunction with gspread-formatting. When my program started to crash I investigated and came up with this issue during debugging. So I suppose your question, which is a good one, could be answered by the developers of packages such as gspread-formatting

[alifeee]

You need the attribute spreadsheet located in a Worksheet instance. This attribute provide you the parent Spreadsheet instance from the current Worksheet.
what is preventing you from using the actual Spreaddsheet instance you previously used to obtain the Worksheet instance ?
if this is can be solved by other means and allow us to remove any cyclic dependency in our code I'm gladly happy to help 🙃

Thanks for the response! From your question I see there is a bit of confusion, I am not myself using the spreadsheet attribute of Worksheet, but rather I am using gspread in conjunction with gspread-formatting. When my program started to crash I investigated and came up with this issue during debugging. So I suppose your question, which is a good one, could be answered by the developers of packages such as gspread-formatting

I have not used or created code for gspread-formatting, but looking at the code I can partly answer this question.

The problem is: a user wants the conditional formatting rules for a worksheet. So, gspread-formatting provides get_conditional_format_rules(worksheet).

It would be unreasonable for gspread-formatting to ask the user provide a Spreadsheet object, as the user is only interested in the conditional formatting of the worksheet. So, you would have to provide get_conditional_format_rules(spreadsheet, worksheet), which I think is bad.

So, get_conditional_format_rules(worksheet) only has access to the worksheet object, but conditional formatting is information provided by Spreadsheet metadata. This is why gspread-formatting needs Worksheet.Spreadsheet.

[lavigne958]

I followed the link and checked the example method.

I found that the method get_conditional_format_rules(worksheet) needs the spreadsheet attribute so it can pull sheet metadata, like you did.

Now the method that pulls sheet metadata is located in our new HTTPClient, and this client is now accessible from the Worksheet object 🥳 which means we can pull a PR in gspread_formating that now uses worksheet.client.fetch_sheet_metadata() ! 🙃💪
And it should work with no user code change

So first thing first, make it work and backward compatible with gspread-6.0.1 then use the new features.

Though we need to check if the whole project can be adjusted not juste that specific function

[alifeee]

I like the enthusiasm!

But, if the typing is fine with the circular import, what is the problem with just leaving the spreadsheetattribute in?

Why do we want to remove Worksheet.spreadsheet?

I think it seems like a useful pattern.

[lavigne958]

I like the enthusiasm!

But, if the typing is fine with the circular import, what is the problem with just leaving the spreadsheetattribute in?

The problem is the circular import.
Having a workaround provided by mypy does mean its a good design/software architecture. 🙃
It makes no sense to have a circular import, so users can use this object, to access it's parent object (that they already have anyway) in order to call pure HTTP methods. We can do this using the http client in the worksheet objet.

I checked the gspread formating package and the only reason it uses the spreadsheet attribute is to call HTTP methods. That was the only option back then, it's not anymore today. So the package can be updated.

Why do we want to remove Worksheet.spreadsheet?

I think it seems like a useful pattern.

To have a better software architecture that removes some issues and work from us, no more circular dependency, that simplifies typing, allows to change, edit, upgrade any part without being blocked by other components, easier to introduce new features etc.