Skip to content

Conversation

@jsignell
Copy link
Member

@jsignell jsignell commented Mar 19, 2021

This fixes the test failure mentioned in #7366 (comment) and adds a non hdfs test for this case.

Ping @martindurant @jrbourbeau

delayed(partial(file_to_blocks, include_path, delimiter=linedelimiter))(
fil
delayed(list)(
delayed(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose we should explicitly test the output of this branch, one list (len files) of lists (blocks per file) of text.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was a change introduced accidentally in#7349 I think.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe, but it didn't fail anything right away - there should be a test for db.read_text(fn, collection=False)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added that in this PR.

@jsignell
Copy link
Member Author

Comment on lines +87 to +96
def test_read_text_unicode_no_collection():
data = b"abcd\xc3\xa9"
fn = "./data.txt"
with open(fn, "wb") as f:
f.write(b"\n".join([data, data]))

f = read_text(fn, collection=False)

result = f[0].compute()
assert len(result) == 2
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

perfect

@jsignell
Copy link
Member Author

Ok I ran hdfs tests on this PR as well to make sure that this fixes the original issue.

@jsignell jsignell merged commit b288de7 into dask:main Mar 19, 2021
@jsignell jsignell deleted the fix-hdfs branch March 19, 2021 18:08
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.

Maybe run HDFS tests based on commit message

2 participants