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

TST: Split test_excel into sub test files #24472

Closed
WillAyd opened this issue Dec 28, 2018 · 6 comments · Fixed by #26755
Closed

TST: Split test_excel into sub test files #24472

WillAyd opened this issue Dec 28, 2018 · 6 comments · Fixed by #26755
Labels
good first issue IO Excel read_excel, to_excel Refactor Internal refactoring of code Testing pandas testing functions or related to the test suite
Milestone

Comments

@WillAyd
Copy link
Member

WillAyd commented Dec 28, 2018

Logical follow up to https://github.com/pandas-dev/pandas/pull/24423/files#r244284618

cc @gfyoung

@WillAyd WillAyd added Testing pandas testing functions or related to the test suite IO Excel read_excel, to_excel labels Dec 28, 2018
@gfyoung gfyoung added Refactor Internal refactoring of code good first issue labels Dec 29, 2018
@stevenbw
Copy link

stevenbw commented Jan 7, 2019

Hi @WillAyd, is this issue still open? If so I would be interested in giving it a shot :)

@WillAyd
Copy link
Member Author

WillAyd commented Jan 7, 2019

@stevenbw for sure - PRs are always welcome!

@stevenbw
Copy link

stevenbw commented Jan 8, 2019

@WillAyd Just want to make sure I am interpreting this correctly. The only change is to move the TestXlrdReader class into its own file from test_excel?

@WillAyd
Copy link
Member Author

WillAyd commented Jan 8, 2019

At the very least want to create a sub directory for excel which has a base.py and an xlrd.py which logically separate the tests. There may be other logical organization points so if you have ideas on that certainly open to it.

Cc @gfyoung if you have any other thoughts to offer

@gfyoung
Copy link
Member

gfyoung commented Jan 8, 2019

@WillAyd : I think you explained it pretty well here!

@stevenbw : Just a point of guidance. The idea is to hopefully create smaller, more modular tests in the spirit of pytest, hence why we would like to split the tests up instead of having one massive file.

There might also be opportunities to leverage more pytest idiom such as fixtures and parameterizations, but we can cross that bridge later if need be.

@stevenbw
Copy link

@WillAyd @gfyoung thanks for your help. I have split the file in two as you suggested, just reviewing before I open a PR. There could be an opportunity to separate the openpyxl tests out.

As @gfyoung said, it may be worthwhile undertaking the bigger task of refactoring the test files to a more functional style rather than class based.

stevenbw added a commit to stevenbw/pandas that referenced this issue Jan 13, 2019
stevenbw added a commit to stevenbw/pandas that referenced this issue Jan 13, 2019
stevenbw added a commit to stevenbw/pandas that referenced this issue Jan 13, 2019
stevenbw added a commit to stevenbw/pandas that referenced this issue Jan 13, 2019
stevenbw added a commit to stevenbw/pandas that referenced this issue Jan 13, 2019
stevenbw added a commit to stevenbw/pandas that referenced this issue Jan 20, 2019
stevenbw added a commit to stevenbw/pandas that referenced this issue Feb 15, 2019
@simonjayhawkins simonjayhawkins added this to the 0.25.0 milestone Jun 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue IO Excel read_excel, to_excel Refactor Internal refactoring of code Testing pandas testing functions or related to the test suite
Projects
None yet
4 participants