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

add guess_max param for guessing col_type; fixes #223 #257

Merged
merged 8 commits into from Feb 11, 2017

Conversation

Projects
None yet
3 participants
@jennybc
Member

jennybc commented Feb 10, 2017

Incorporates and extends @tklebel's PR re: adding a parameter to control number of rows used for col type guessing. I did the rebase locally and closed the original PR.

Goal was to make everything as close to readr as possible. read_excel() does not (yet?) have an n_max parameter, so that's a small difference.

Why now? Turns out the column type stuff is easier to work on and test if guess_max is exposed, so I got this out of the way.

@jennybc jennybc changed the title from add new param n for guessing col_type to add new param n for guessing col_type; fixes #223 Feb 10, 2017

@hadley

This comment has been minimized.

Member

hadley commented Feb 10, 2017

(Most of this diff seems to be a spurious re-ordering of the stuff that Rcpp generates; I've seen this before but I can't remember what causes it)

@jennybc

This comment has been minimized.

Member

jennybc commented Feb 11, 2017

parseXml and countRows re-situate themselves perpetually 🙂🙃🙂🙃 and I don't know why.

I asked @jimhester about it but he's not familiar with the problem. I try to NOT commit these but apparently I did this time. The guilty commit is the rebase, which was a real PIA, so I'm going to just let it be.

@jennybc jennybc requested a review from hadley Feb 11, 2017

@jennybc jennybc changed the title from add new param n for guessing col_type; fixes #223 to add new param guess_max for guessing col_type; fixes #223 Feb 11, 2017

@jennybc jennybc changed the title from add new param guess_max for guessing col_type; fixes #223 to add guess_max param for guessing col_type; fixes #223 Feb 11, 2017

@hadley

hadley approved these changes Feb 11, 2017

@jennybc jennybc merged commit 12dad81 into tidyverse:master Feb 11, 2017

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@jennybc jennybc deleted the jennybc:tklebel-PR-n-for-col-type-guessing branch Feb 11, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment