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: Add another test for uneven CSV rows #15114

Merged

Conversation

gfyoung
Copy link
Member

@gfyoung gfyoung commented Jan 12, 2017

Title is self-explanatory.

xref #15066.
Closes #9549.

@codecov-io
Copy link

codecov-io commented Jan 12, 2017

Current coverage is 84.75% (diff: 100%)

Merging #15114 into master will not change coverage

@@             master     #15114   diff @@
==========================================
  Files           145        145          
  Lines         51232      51232          
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
  Hits          43420      43420          
  Misses         7812       7812          
  Partials          0          0          

Powered by Codecov. Last update 0fe491d...303c340

@cpcloud
Copy link
Member

cpcloud commented Jan 12, 2017

@gfyoung Is this kind of CSV currently untested?

@gfyoung
Copy link
Member Author

gfyoung commented Jan 12, 2017

It is, but to be safe we always include test cases from issues we close.

@cpcloud cpcloud added IO CSV read_csv, to_csv Testing pandas testing functions or related to the test suite labels Jan 12, 2017
@jorisvandenbossche
Copy link
Member

It is, but to be safe we always include test cases from issues we close.

Indeed, we do, but in this case it does seem like almost exactly the same test as the one above but with different numbers? (only usage of header or not, but I don't think this should matter?)

@jorisvandenbossche jorisvandenbossche added this to the 0.20.0 milestone Jan 12, 2017
@gfyoung
Copy link
Member Author

gfyoung commented Jan 12, 2017

@jorisvandenbossche : It shouldn't, but more tests couldn't hurt 😄

@jorisvandenbossche
Copy link
Member

@gfyoung I also changed the 'closes ..', as I would like to keep the issue open for the discussion about the bad_lines keyword in there. Will update the title over there

@gfyoung
Copy link
Member Author

gfyoung commented Jan 12, 2017

@jorisvandenbossche : I'm changing it back because #5986 and #9729 are better places to have this IMO (we should de-duplicate discussions if possible).

@jorisvandenbossche
Copy link
Member

more tests couldn't hurt

more tests start to hurt when we don't see the wood for the trees anymore

@jorisvandenbossche jorisvandenbossche merged commit 66c0702 into pandas-dev:master Jan 12, 2017
@jorisvandenbossche
Copy link
Member

And changed in back again :-)
#9729 is about too few fields, not about processing rows with too many.
#5986 is probably a wrong link?

But maybe its better to just open a new issue about it and refer to the other ones.

@gfyoung
Copy link
Member Author

gfyoung commented Jan 12, 2017

@jorisvandenbossche : Meant #5686 (sorry!)

I don't fully understand what you said there (might be the forest-trees idiom)

@gfyoung gfyoung deleted the unclean-csv-test-confirm branch January 12, 2017 22:37
@jorisvandenbossche
Copy link
Member

jorisvandenbossche commented Jan 12, 2017

I don't fully understand what you said there (might be the forest-trees idiom)

yet I looked it up before posting to be sure it was correct English :-) http://dictionary.cambridge.org/dictionary/english/not-see-the-wood-for-the-trees. With too many tests, it is more difficult to keep the overview

AnkurDedania pushed a commit to AnkurDedania/pandas that referenced this pull request Mar 21, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
IO CSV read_csv, to_csv 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