Repository navigation
Fix read_file_multiprocessing returning duplicated rows when fewer rows than processes are read - #349
Merged
ofajardo merged 2 commits intoOct 13, 2026
Conversation
… rows When the number of rows to read is smaller than num_processes (for example row_limit=2 with 4 processes, or read_file_in_chunks with multiprocess=True and a chunksize smaller than num_processes), some workers were assigned a chunk of 0 rows. That 0 was passed as row_limit to the read function, where 0 means "no limit", so each of those workers read every remaining row of the file and the result contained duplicated rows. Chunks with 0 rows are now skipped. If all chunks are empty one is kept so that an empty dataframe with the right columns is still returned.
Collaborator
|
Good catch and good fix, thanks! Tests are failing due to introduction of polars 2.0 I just fixed it on dev. Therefore, please update your branch with the latests changes on dev. If all tests pass here on github, then I merge. |
Contributor
Author
|
Thanks! I merged the latest |
Collaborator
|
Thanks! |
This file contains hidden or 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
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.
read_file_multiprocessingsplits the rows to read acrossnum_processesworkers. When fewer rows are requested than there are processes, some workers get a share of 0 rows. That 0 is passed to the read function asrow_limit=0, which means "no limit", so those workers read every remaining row and the result has many duplicated rows.Example with
test_data/basic/sample_large.sav(485 rows):The same happens with
read_file_in_chunks(..., multiprocess=True)whenchunksizeis smaller thannum_processes, and withread_file_multiprocessing(..., row_limit=0), which returnednum_processescopies of the file.The fix skips chunks with 0 rows. If all chunks are empty (empty file, or offset at or past the end), one is kept so an empty dataframe with the right columns is still returned, as before.
I added a test for each case to
test_basic.pyandtest_narwhalified.py. They fail ondevand pass with the fix. The full test_basic, test_narwhalified (pandas and polars) and test_runtime_types suites pass.Row counts on
sample_large.sav(485 rows) withnum_processes=4:devrow_limit=2row_limit=0With this change, an explicit
row_limit=0reads the file in a single worker. If you'd like it to stay parallel, treatingrow_limit=0asfloat("inf")at the top of the function would do it; I kept this PR to one change.