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

Export only one worksheet by it's Id from all document added #1491

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

ShevchenkoVadim
Copy link

Added the ability to export one page from the entire document by ID

gspread/http_client.py Outdated Show resolved Hide resolved
gspread/http_client.py Show resolved Hide resolved
gspread/http_client.py Show resolved Hide resolved
@alifeee
Copy link
Collaborator

alifeee commented Jul 18, 2024

you can run these commands to check that the workflow will pass :)

pip install -r lint-requirements.txt
tox -e lint
# optionally but you have not changed tests/docs
pip install -r docs/requirements.txt
pip install -r test-requirements.txt
tox -e doc # build docs
tox -e py # run tests

is this a feature that you think is possible to add a test for, or not?

Copy link
Collaborator

@lavigne958 lavigne958 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the list of possible formats are slightly different from the export format from the DriveAPI.
I suggest we create a new Enum list with the appropriate Names and Values.

I am not too keen on introducing this feature for one reason:

  • in any error scenario we receive an HTML error page.
  • the whole API is made to handle JSON response in all error scenarios, so here we fail to read the JSON error response, because its HTML format.
  • This is because the URL we use is originally made for humans to use on a web browser.

@alifeee what do you think of this detail ?

Comment on lines +391 to +393
format_str = "pdf"
if format == ExportFormat.PDF:
format_str = "pdf"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if the default value is PDf, then you don't have to check if it's PDF in format

gspread/http_client.py Show resolved Hide resolved
Comment on lines +396 to +397
elif format == ExportFormat.ZIPPED_HTML:
format_str = "zip"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this one does not work.
as well as you don't handle the value: OPEN_OFFICE_SHEET which results as PDF due to default value set at the start of the ifs here.

@alifeee
Copy link
Collaborator

alifeee commented Jul 23, 2024

@alifeee what do you think of this detail ?

I'm not fully sure why this feature is desired. What can gspread currently do, and what is missing?

Could someone provide a (pseudo)code snippet example of what is desired with this function?

@lavigne958
Copy link
Collaborator

lavigne958 commented Jul 23, 2024

@alifeee what do you think of this detail ?

I'm not fully sure why this feature is desired. What can gspread currently do, and what is missing?

I believe it's desired when you wish to export (to PDF, to CSV, ...) a single sheet instead of a the whole spreadsheet (which creates multiple pages PDF for example).

Could someone provide a (pseudo)code snippet example of what is desired with this function?

sure, I tried it, here it is 🙃

client = gspread.service_account()
file = client.open("xxxxxxxx")
res = file.sheet1.export(gspread.utils.ExportFormat.PDF)
with open("result.pdf", "wb") as fp:
    fp.write(res)

My major concerns are:

  1. the error handling, which returns HTML code, and not a JSON
  2. the exported format that are not fully covered in this MR. (this is easily fixed with a new Enum)

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

Successfully merging this pull request may close these issues.

3 participants