Skip to content

REF: Read excel parse refactor #58497

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 28 commits into from
May 1, 2024
Merged

REF: Read excel parse refactor #58497

merged 28 commits into from
May 1, 2024

Conversation

iangainey
Copy link
Contributor

@iangainey iangainey commented Apr 30, 2024 •
edited
Loading

This is related to PR 58464 as a precursor to that enhancement

@iangainey iangainey requested a review from rhshadrach as a code owner April 30, 2024 16:19
rhshadrach
rhshadrach requested changes Apr 30, 2024
View reviewed changes
Copy link
Member

@rhshadrach rhshadrach left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! Looks great - a rename request (I had forgotten the _ when I made the recommendation on the other PR).

pandas/io/excel/_base.py Outdated

for col in index_col:
last = data[offset][col]
def parse_sheet(
Copy link
Member

Choose a reason for hiding this comment

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

Since this class is in the public API, we'll want to mark this internal. Can you rename to _parse_sheet.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Of course, I've corrected it. Thank you for taking a loot at it. Let me know if there's anything else that can be changed or improved in this PR!

@rhshadrach rhshadrach changed the title Read excel parse refactor REF: Read excel parse refactor Apr 30, 2024
@rhshadrach rhshadrach added Refactor Internal refactoring of code IO Excel read_excel, to_excel labels Apr 30, 2024
@rhshadrach rhshadrach added this to the 3.0 milestone Apr 30, 2024
@rhshadrach rhshadrach mentioned this pull request Apr 30, 2024
Closed
5 tasks
rhshadrach
rhshadrach approved these changes May 1, 2024
View reviewed changes
Copy link
Member

@rhshadrach rhshadrach left a comment

Choose a reason for hiding this comment

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

lgtm

@rhshadrach rhshadrach merged commit f6932cb into pandas-dev:main May 1, 2024
46 checks passed
@rhshadrach
Copy link
Member

Thanks @iangainey!

pmhatre1 pushed a commit to pmhatre1/pandas-pmhatre1 that referenced this pull request May 7, 2024
@iangainey @pmhatre1
REF: Read excel parse refactor (pandas-dev#58497)
37f02ab
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rhshadrach rhshadrach rhshadrach approved these changes

Assignees
No one assigned
Labels
IO Excel read_excel, to_excel Refactor Internal refactoring of code
Projects
None yet
Milestone
3.0
Development

Successfully merging this pull request may close these issues.
2 participants
@iangainey @rhshadrach