Add missing method import_csv()

[lavigne958]

Prior to v6.x.y the method could be used as follow:

file = client.open('x')
file.client.import_csv()

Now file.client is of type HTTPClient
so we need to add the method back for backward compatibility

closes #1423

[alifeee]

This looks good. I only note:

I don't think import_csv makes sense within HTTP_Client? This method provides spreadsheet functionality, so it should be in gspread.Client?

I am not fully up to date with the idea of the HTTP_Client, but my understanding is that it was made so that people could make their own custom HTTP_Clients easier. Then, they would have to make their own import_csv method? Should this not belong to Client?

I see that it might have to be in HTTP_Client, because of hierarchical reasons. If this is the case, then I guess we must leave it.

[lavigne958]

This looks good. I only note:

I don't think import_csv makes sense within HTTP_Client? This method provides spreadsheet functionality, so it should be in gspread.Client?

I am not fully up to date with the idea of the HTTP_Client, but my understanding is that it was made so that people could make their own custom HTTP_Clients easier. Then, they would have to make their own import_csv method? Should this not belong to Client?

I see that it might have to be in HTTP_Client, because of hierarchical reasons. If this is the case, then I guess we must leave it.

I know 😞 I faced the same thought, I wish it could only be located in the client itself and only be used like: gs_client.import_csv() which returns the new spreadsheet instance.

I agree, the HTTPClient allows users to make their own if necessary, and mostly it is here to handle only pure HTTP calls, on the least amount of logic.

The way the project was designed before, allowed users to reach the argument client in a spreadsheet and call methods on it, like this one. now that client argument is the HTTPClient so some logic has been removed.

what we can do is:

• leave the function only in the gspread client, like it is now
• do not put it back in the HTTPClient

which is in the end: make no changes, as currently the documentation of the function shows that you should use that function using the gspread client directly, it never shows that you can reach it from elsewhere.

or add it back to the HTTPClient so users can do: spreadsheet.client.import_csv() but we put back the deprecation warnings, in order to be very clean. and we live with it until next major release 🤔

[alifeee]

or add it back to the HTTPClient so users can do: spreadsheet.client.import_csv() but we put back the deprecation warnings, in order to be very clean. and we live with it until next major release 🤔

This is a good question. I er towards adding it to the HTTPClient. As, in the end, if someone wanted to make their own, they would probably create a sub-class of the HTTPClient, and override the request methods. So the import_csv etc methods would remain.

Maximum backwards compatibility :)

[lavigne958]

or add it back to the HTTPClient so users can do: spreadsheet.client.import_csv() but we put back the deprecation warnings, in order to be very clean. and we live with it until next major release 🤔

This is a good question. I er towards adding it to the HTTPClient. As, in the end, if someone wanted to make their own, they would probably create a sub-class of the HTTPClient, and override the request methods. So the import_csv etc methods would remain.

Maximum backwards compatibility :)

agreed, we can later depreciate it, like before for the v6.0.0, but this time we should have way less methods to depreciate ! so it should be much easier.

[lavigne958]

thinking about it, I have no changes to make currently right ? unless I forgot something.

[alifeee]

something must be changed to fix #1423, no?

[lavigne958]

the current pull request will fix it.
The original issue mentions: AttributeError: 'HTTPClient' object has no attribute 'import_csv'

and the pull request adds a new method import_csv to the HTTPClient.

I am waiting for your review/approval mostly now that we agree on leaving the new method. 🙃

Unless: I did not understand and we still need to find a solution, which is fine.