Skip to content

Conversation

@z764969689
Copy link
Contributor

gh-57531 add BufferedReader.read() return value check for non-blocking stream mode, simply opt the TextIOWrapper error raising.

@serhiy-storchaka
Copy link
Member

Please resolve conflicts and add tests. Also capitalize the first letter of a sentence in a NEWS entry.

@z764969689
Copy link
ContributorAuthor

z764969689 commented Jan 18, 2024

@serhiy-storchaka Conflicts resolved & NEWS entry edited. I'm wondering if I need to open another PR for the tests or just add them in this one.

@serhiy-storchaka
Copy link
Member

It is better to add new tests in the same PR. It makes easier to check that they fail without this change and pass with it.

@z764969689
Copy link
ContributorAuthor

I've added a simple test case and I found that the test case would fail the C implementation. Since I'm unfamiliar with the C code, might need some help from others to modify the C code.
BTW, not pretty clear on the implementation differences, is it okay that the error-raising logic varies between them?

@serhiy-storchaka
Copy link
Member

It is more complex issue than covered in this PR. If change the behavior of text file read() for non-blocking buffered readers, the same change should be done for non-blocking raw files. If change the behavior for "read-until-end", the same change should be done for partial read.

And of course the behavior of both C and Python implementation should be consistent.

Thank you for reviving this issue, I will look what can I do with this.

@z764969689z764969689 deleted the fix-issue-57531 branch January 30, 2024 07:44
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@z764969689@serhiy-storchaka@bedevere-bot