Fix CSV.Rows when missing values are parsed #904
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Fixes #903. The issue here is a consequence of the big internal refactoring that happened for the 0.9 release. Part of the refactoring is that `parserow` doesn't explicitly set a column's value to `missing` if a sentinel value is detected while parsing. This is because when a column is allocated, it's allocated with "all missing" values set, so it _should_ already be set to missing. This doesn't hold up, however, for `CSV.Rows`, where we only allocate a single element "column" vector for each column and reuse it when iterating rows. The solution, therefore, is to "reset" the column vectors before each call to `parserow` to ensure the column values are set to missing in case a sentinel value is detected while parsing.
Codecov Report
@@ Coverage Diff @@
## main #904 +/- ##
==========================================
- Coverage 90.56% 89.68% -0.89%
==========================================
Files 9 9
Lines 2162 2181 +19
==========================================
- Hits 1958 1956 -2
- Misses 204 225 +21
Continue to review full report at Codecov.
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #903. The issue here is a consequence of the big internal
refactoring that happened for the 0.9 release. Part of the refactoring
is that
parserowdoesn't explicitly set a column's value tomissingif a sentinel value is detected while parsing. This is because when a
column is allocated, it's allocated with "all missing" values set, so it
should already be set to missing. This doesn't hold up, however, for
CSV.Rows, where we only allocate a single element "column" vector foreach column and reuse it when iterating rows. The solution, therefore,
is to "reset" the column vectors before each call to
parserowtoensure the column values are set to missing in case a sentinel value is
detected while parsing.