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

fread NUL follow up #3505

Merged
merged 4 commits into from
Apr 15, 2019
Merged

fread NUL follow up #3505

merged 4 commits into from
Apr 15, 2019

Conversation

mattdowle
Copy link
Member

@mattdowle mattdowle commented Apr 12, 2019

Follow up to #3433 for #3400
Closes #3400
Closes #2435

  • Added news item and Roy to contributor list.
  • the file added for test 2025 has an extra line of data after the NUL. It stops on the NUL silently currently. I'd lean towards skipping the NUL with warning and then proceeding as if the NUL wasn't there, which in this case would reading the extra line of data. Note the difference between cat and more :
$ more issue_3400_fread.txt 
a=b
A  B  C
1  2  3
3  2  1
$ cat issue_3400_fread.txt 
a=b
A  B  C
1  2  3
3  2  1
4  5  6

@mattdowle mattdowle added this to the 1.12.4 milestone Apr 12, 2019
@codecov
Copy link

codecov bot commented Apr 13, 2019

Codecov Report

Merging #3505 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #3505   +/-   ##
=======================================
  Coverage   96.66%   96.66%           
=======================================
  Files          65       65           
  Lines       12277    12277           
=======================================
  Hits        11867    11867           
  Misses        410      410

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 b84d0fe...2fb8aa2. Read the comment docs.

@codecov
Copy link

codecov bot commented Apr 13, 2019

Codecov Report

Merging #3505 into master will decrease coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3505      +/-   ##
==========================================
- Coverage   96.66%   96.66%   -0.01%     
==========================================
  Files          65       65              
  Lines       12280    12278       -2     
==========================================
- Hits        11870    11868       -2     
  Misses        410      410
Impacted Files Coverage Δ
src/fread.c 98.49% <100%> (-0.01%) ⬇️

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 392d6df...3507f03. Read the comment docs.

@mattdowle mattdowle merged commit 96543dd into master Apr 15, 2019
@mattdowle mattdowle deleted the freadNULL branch April 15, 2019 21:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[bug] fread handling embedded NUL characters fread error reading Latin-1 file containing NUL byte <0x00>
1 participant