Split test_excel into submodule #26755

WillAyd · 2019-06-09T15:18:50Z

closes TST: Split test_excel into sub test files #24472

Just moved everything as is. Some of the class structures are unnecessary now but leaving for follow ups

pandas/tests/io/excel/conftest.py

WillAyd · 2019-06-09T15:20:13Z

pandas/tests/io/excel/conftest.py

+
+
+@pytest.fixture
+def df_ref():


Similar comment as above but this would only be used in the reader tests

WillAyd · 2019-06-09T15:21:19Z

pandas/tests/io/excel/conftest.py

+
+
+@pytest.fixture(params=[True, False])
+def merge_cells(request):


Also only used in writers

codecov · 2019-06-09T15:55:06Z

Codecov Report

Merging #26755 into master will decrease coverage by <.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master   #26755      +/-   ##
==========================================
- Coverage    91.7%   91.69%   -0.01%     
==========================================
  Files         179      179              
  Lines       50767    50767              
==========================================
- Hits        46555    46552       -3     
- Misses       4212     4215       +3

Flag	Coverage Δ
#multiple	`90.29% <ø> (ø)`	⬆️
#single	`41.22% <ø> (-0.07%)`	⬇️

Impacted Files	Coverage Δ
pandas/io/gbq.py	`78.94% <0%> (-10.53%)`	⬇️
pandas/core/frame.py	`96.88% <0%> (-0.12%)`	⬇️
pandas/util/testing.py	`90.73% <0%> (+0.1%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c7748ca...0f9ceaf. Read the comment docs.

codecov · 2019-06-09T15:55:07Z

Codecov Report

Merging #26755 into master will decrease coverage by <.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master   #26755      +/-   ##
==========================================
- Coverage    91.7%   91.69%   -0.01%     
==========================================
  Files         179      178       -1     
  Lines       50767    50758       -9     
==========================================
- Hits        46555    46543      -12     
- Misses       4212     4215       +3

Flag	Coverage Δ
#multiple	`90.29% <ø> (-0.01%)`	⬇️
#single	`41.21% <ø> (-0.08%)`	⬇️

Impacted Files	Coverage Δ
pandas/io/gbq.py	`78.94% <0%> (-10.53%)`	⬇️
pandas/core/frame.py	`96.88% <0%> (-0.12%)`	⬇️
pandas/io/excel/__init__.py
pandas/util/testing.py	`90.73% <0%> (+0.1%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c7748ca...d2a5d3a. Read the comment docs.

simonjayhawkins

@WillAyd lgtm.

does this now close #24472?

it doesn't seem to affect test discoverabily but should there be an __init__.py? I think all the other test directories have them.

is there going to be a further split of test_writers.py and test_readers.py?

pandas/tests/io/excel/conftest.py

WillAyd · 2019-06-09T16:56:36Z

Yea I think the init is a good idea to preserve the namespacing so just pushed. Also updated OP to reflect closure of issue

simonjayhawkins

Thanks @WillAyd . Nice!

jreback

so

you can use pytest.importorskip here and directly import things
If you can not use the class structure I would do it. Since this changes lots of code I would do it in this PR.

jreback · 2019-06-09T21:40:46Z

pandas/tests/io/excel/test_openpyxl.py

+from pandas.io.excel import ExcelWriter, _OpenpyxlWriter
+
+
+@td.skip_if_no('openpyxl')


use a pytest.importorskip

also I would not use classes unless you have separate discrete cases

jreback · 2019-06-09T21:41:24Z

pandas/tests/io/excel/test_readers.py

+
+        tm.assert_frame_equal(expected, actual)
+
+    @td.skip_if_no('py.path')


can remove this test entirely

jreback · 2019-06-09T21:42:48Z

pandas/tests/io/excel/test_readers.py

+
+class TestReaders:
+
+    @pytest.fixture(autouse=True, params=[


can these just be module pytest.importorskip?

jreback · 2019-06-09T21:43:52Z

pandas/tests/io/excel/test_writers.py

+    register_writer)
+
+
+@td.skip_if_no('xlrd')


same comment as above

jreback · 2019-06-09T21:44:18Z

pandas/tests/io/excel/test_xlrd.py

+from pandas.io.excel import ExcelFile
+
+
+@td.skip_if_no('xlrd')


same comment

simonjayhawkins · 2019-06-09T21:50:01Z

@jreback IIUC this is just a plain split and move, no changes. your comments will be addressed in follow-ups.

jreback · 2019-06-09T21:50:54Z

ok that's fine

simonjayhawkins · 2019-06-09T22:37:50Z

@WillAyd I know it was a lot of work to be able to get to this point. If you now want to focus and push on with #25092 and #25427, then i'll be happy to carry-on with the clean-up and address @jreback comments.

WillAyd · 2019-06-10T02:27:43Z

@simonjayhawkins that would be great - thanks for offering!

WillAyd added 2 commits June 9, 2019 11:14

Split test_excel into submodule

aeace8c

Applied isort

0f9ceaf

WillAyd added Testing pandas testing functions or related to the test suite IO Excel read_excel, to_excel labels Jun 9, 2019

WillAyd commented Jun 9, 2019

View reviewed changes

simonjayhawkins reviewed Jun 9, 2019

View reviewed changes

pandas/tests/io/excel/conftest.py Show resolved Hide resolved

Added init file

d2a5d3a

simonjayhawkins approved these changes Jun 9, 2019

View reviewed changes

simonjayhawkins added this to the 0.25.0 milestone Jun 9, 2019

jreback requested changes Jun 9, 2019

View reviewed changes

jreback approved these changes Jun 9, 2019

View reviewed changes

jreback merged commit 9413591 into pandas-dev:master Jun 9, 2019

WillAyd deleted the xl-test-submod branch June 10, 2019 02:27

This was referenced Jun 10, 2019

TST/CLN: declass smaller test files in tests\io\excel #26764

Merged

Add more pytest idiom to io.excel testing #26784

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Split test_excel into submodule #26755

Split test_excel into submodule #26755

WillAyd commented Jun 9, 2019 •

edited

Loading

WillAyd Jun 9, 2019

WillAyd Jun 9, 2019

codecov bot commented Jun 9, 2019

codecov bot commented Jun 9, 2019 •

edited

Loading

simonjayhawkins left a comment

WillAyd commented Jun 9, 2019

simonjayhawkins left a comment

jreback left a comment

jreback Jun 9, 2019

jreback Jun 9, 2019

jreback Jun 9, 2019

jreback Jun 9, 2019

jreback Jun 9, 2019

simonjayhawkins commented Jun 9, 2019

jreback commented Jun 9, 2019

simonjayhawkins commented Jun 9, 2019

WillAyd commented Jun 10, 2019



		@pytest.fixture(params=[True, False])
		def merge_cells(request):

		from pandas.io.excel import ExcelWriter, _OpenpyxlWriter


		@td.skip_if_no('openpyxl')


		tm.assert_frame_equal(expected, actual)

		@td.skip_if_no('py.path')

		register_writer)


		@td.skip_if_no('xlrd')

Split test_excel into submodule #26755

Split test_excel into submodule #26755

Conversation

WillAyd commented Jun 9, 2019 • edited Loading

WillAyd Jun 9, 2019

Choose a reason for hiding this comment

WillAyd Jun 9, 2019

Choose a reason for hiding this comment

codecov bot commented Jun 9, 2019

Codecov Report

codecov bot commented Jun 9, 2019 • edited Loading

Codecov Report

simonjayhawkins left a comment

Choose a reason for hiding this comment

WillAyd commented Jun 9, 2019

simonjayhawkins left a comment

Choose a reason for hiding this comment

jreback left a comment

Choose a reason for hiding this comment

jreback Jun 9, 2019

Choose a reason for hiding this comment

jreback Jun 9, 2019

Choose a reason for hiding this comment

jreback Jun 9, 2019

Choose a reason for hiding this comment

jreback Jun 9, 2019

Choose a reason for hiding this comment

jreback Jun 9, 2019

Choose a reason for hiding this comment

simonjayhawkins commented Jun 9, 2019

jreback commented Jun 9, 2019

simonjayhawkins commented Jun 9, 2019

WillAyd commented Jun 10, 2019

WillAyd commented Jun 9, 2019 •

edited

Loading

codecov bot commented Jun 9, 2019 •

edited

Loading