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

Allow reading and writing Excel files in remote sources #506

Closed
wants to merge 4 commits into from

Conversation

juarezr
Copy link
Member

@juarezr juarezr commented Jul 20, 2020

Today the excel implementation only allows reading and writing Excel files in the local filesystem.
Other formats like avro, csv, json, text already support reading from remote servers/cloud using fsspec package.
This improves the current implementation for allowing opening remote xls and xlsx files when using fsspec.

Changes

  1. Changed fromxls() and toxls() to call read_source_from_arg or write_source_from_arg.
  2. Changed fromxlsx(), toxlsx() and appendxlsx() to call read_source_from_arg or write_source_from_arg.
  3. Added tests for testing remote testing of the xls and xlsx file formats.
  4. Added appendxlsx() to init.py making it public
  5. Tested with S3 remote source.

Checklist

Checklist for for pull requests including new code and/or changes to existing code...

  • Code
    • Includes unit tests
    • New functions have docstrings with examples that can be run with doctest
    • New functions are included in API docs
    • Docstrings include notes for any changes to API or behaviour
    • All changes documented in docs/changes.rst
  • Testing
    • (Optional) Tested local against remote servers
    • Travis CI passes (unit tests run under Linux)
    • AppVeyor CI passes (unit tests run under Windows)
    • Unit test coverage has not decreased (see Coveralls)
  • Changes
    • Ready to review
    • Ready to merge

@coveralls
Copy link

coveralls commented Jul 20, 2020

Coverage Status

Coverage decreased (-0.1%) to 91.781% when pulling 3293c68 on juarezr:xlsx_remotes into d713928 on petl-developers:master.

@juarezr
Copy link
Member Author

juarezr commented Aug 6, 2020

This PR conflicts with #502 because it changes the same lines of code for Excel support.

So I thinking on:

  1. merging this PR
  2. after this PR is merged rework Allow toxlsx() to add or replace a worksheet. #502
  3. test, review and merge the changes in Allow toxlsx() to add or replace a worksheet. #502 reworked

Any suggestions?

@juarezr
Copy link
Member Author

juarezr commented Aug 13, 2020

Closing this PR because it's changes were merged in #509.

@juarezr juarezr closed this Aug 13, 2020
@juarezr juarezr deleted the xlsx_remotes branch September 17, 2020 14:57
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

2 participants