-
Notifications
You must be signed in to change notification settings - Fork 5
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
Update defaults for reading files #86
Conversation
@suvayu This first change is the naive approach: updating the default and, consequently, the files and the tests. I think you want something more robust, but I'm unsure how to do it. If you give me some hints, I can work on it. Thanks! |
01eeb37
to
595cae5
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #86 +/- ##
=======================================
Coverage 66.91% 66.91%
=======================================
Files 6 6
Lines 266 266
=======================================
Hits 178 178
Misses 88 88 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see a better way to do this either.
What I like about it now:
- only one (unsurprising) setting, header on/off.
- can be easily overridden by passing
;header = false, other = val
to any of thecreate_tbl
functions, along with any other options
What I don't like: it lives in an internal pair in that module.
I won't mind it much after this PR because it doesn't do anything surprising. Previously it was by skipping lines. So it was in a way a consequence of an odd choice we made for our data files.
You can actually see the consequence in your PR, removing that |
Changing the default not to skip rows when reading
Related issues
Closes #85
Checklist