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

fix uploading header twice if DataFrame is empty, fixes #47 #48

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

Conversation

22764636
Copy link

@22764636 22764636 commented Nov 8, 2019

returns an error if trying to upload an empty DataFrame to GSheets rather than adding two rows with the DataFrame header

@coveralls
Copy link

Coverage Status

Coverage decreased (-1.2%) to 28.037% when pulling 8c4da47 on 22764636:empty_dataframe into f4cef38 on maybelinot:master.

@maybelinot
Copy link
Owner

Sorry for not looking closer into the issue, I think better behaviour would be to upload just columns. empty dataframe is a valid input and I don't think raising an error is correct way to handle it.

@22764636
Copy link
Author

I agree it would be a better way of handling an empty dataframe however it requires quite a few amendments to your code. If you're comfortable with this I will go ahead and work on the code

@maybelinot
Copy link
Owner

Yes, I’m okay with it, ping me if something isn’t clear

@22764636
Copy link
Author

I think I have fixed it, there was an issue with row_names = True if DataFrame was empty too, I should have solved both issues

However before pushing the updates want to ask if there was any particular reason you created the clean_worksheet function instead of simply calling wks.clear()? I think it is more efficient compared to calling a different function.

@maybelinot
Copy link
Owner

Hi @22764636,

Yes, there was a reason, the idea was to store multiple tables on the same worksheet and being able to clean them up individually, I'd probably want to keep that functionality

@22764636
Copy link
Author

just committed a new update, see if it fits your code

@maybelinot
Copy link
Owner

maybelinot commented Dec 18, 2019 via email

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.

None yet

3 participants