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

CLN: Removed duplicated test data #34477

Merged
merged 6 commits into from
Jun 8, 2020
Merged

Conversation

alimcmaster1
Copy link
Member

Remove duplicated test data.

"test1.csv" & "tips.csv" are present in both "tests/io/parser/data" & "tests/io/data/csv".

Move the ".csv.bz2" & ".csv.gz" files since the s3_resource fixture in tests/io/conftest requires these.

Follow up from: #29439

Think we should move all the data in io/parser/data/* into io/data/ so it can be re-used. Will save for follow up.

@MarcoGorelli
Copy link
Member

Hey @alimcmaster1

Is part of this already addressed in #34458?

@alimcmaster1
Copy link
Member Author

Hey @alimcmaster1

Is part of this already addressed in #34458?

Morning, thanks for pointing this out. I think we will just want to remove pandas/tests/data in that PR? Whereas here I was looking at io/parser/data and io/data directories here

@alimcmaster1 alimcmaster1 changed the title CLN: Removed duplicated test data WIP: CLN: Removed duplicated test data May 30, 2020
@alimcmaster1 alimcmaster1 added the Testing pandas testing functions or related to the test suite label Jun 6, 2020
@alimcmaster1 alimcmaster1 changed the title WIP: CLN: Removed duplicated test data CLN: Removed duplicated test data Jun 6, 2020
@alimcmaster1
Copy link
Member Author

@WillAyd you able to review this? Thanks!

@@ -53,11 +53,11 @@ def csv_dir_path(datapath):


@pytest.fixture
def csv1(csv_dir_path):
Copy link
Member

Choose a reason for hiding this comment

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

What's the reason for doing this instead of editing csv_dir_path?

Copy link
Member Author

Choose a reason for hiding this comment

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

csv_dir_path is used across quite a few tests. We would have to move all csvs from io/parser/data to io/data/csv if we wanted to do this.

Copy link
Member

Choose a reason for hiding this comment

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

Gotcha. I think makes sense to do that but can be done as a follow up

@WillAyd WillAyd added this to the 1.1 milestone Jun 6, 2020
Copy link
Member

@WillAyd WillAyd left a comment

Choose a reason for hiding this comment

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

lgtm @jreback

@jreback jreback merged commit 2e89508 into pandas-dev:master Jun 8, 2020
@jreback
Copy link
Contributor

jreback commented Jun 8, 2020

thanks @alimcmaster1 very nice

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Clean Testing pandas testing functions or related to the test suite
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants