Allow multiple separators in Stream.readuntil

@asvetlov

BPO	37141
Nosy	@asvetlov, @1st1, @bmerry, @websurfer5
PRs	gh-81322: support multiple separators in Stream.readuntil #16429

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = None
created_at = <Date 2019-06-03.09:32:30.680>
labels = ['type-feature', '3.9', 'expert-asyncio']
title = 'Allow multiple separators in Stream.readuntil'
updated_at = <Date 2019-09-26.19:48:53.882>
user = 'https://github.com/bmerry'

bugs.python.org fields:

activity = <Date 2019-09-26.19:48:53.882>
actor = 'yselivanov'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['asyncio']
creation = <Date 2019-06-03.09:32:30.680>
creator = 'bmerry'
dependencies = []
files = []
hgrepos = []
issue_num = 37141
keywords = ['patch']
message_count = 11.0
messages = ['344397', '344398', '344399', '344403', '344404', '344405', '344409', '352089', '352179', '353323', '353335']
nosy_count = 4.0
nosy_names = ['asvetlov', 'yselivanov', 'bmerry', 'Jeffrey.Kintscher']
pr_nums = ['16429']
priority = 'normal'
resolution = 'later'
stage = 'patch review'
status = 'pending'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue37141'
versions = ['Python 3.9']

Text-based protocols sometimes allow a choice of newline separator - I work with one that allows either \r or \n. Unfortunately that doesn't work with StreamReader.readuntil, which only accepts a single separator, so I've had to do some hacky things to obtain lines without having to

From discussion in bpo-32052, it sounded like extending StreamReader.readuntil to support a tuple of separators would be feasible.

Would you make a PR?
I guess to modify Stream.readuntil()
StreamReader is deprecated, I'm not sure if we should add new functionality to this class.

I wasn't aware of that deprecation - it doesn't seem to be mentioned at https://docs.python.org/3.8/library/asyncio-stream.html. What is the replacement?

Docs will be updated soon.
The change has landed a week ago, I had no time for docs update before 3.8 beta. Sorry for that.

The idea is: StreamReader and StreamWriter are merged into just Stream, open_connection() is replaced with connect() etc.

Ok. Does the new Stream still have a similar interface for readuntil i.e. is this still a relevant request against the new API? I'm happy to let deprecated APIs stay as-is.

Yes, Stream supports all StreamReader and StreamWriter methods

Ok, I've changed the issue title to refer to Stream. Since this would be a new feature, I assume it's off the table for 3.8, but I'll see if I get time to implement a PR in time for 3.9 (and get someone at work to sign off on the contributor agreement, which might be the harder part).

Thanks for the quick and helpful responses.

I finally have permission from my employer to sign the contributors agreement, so I'll take a stab at this when I have some free time (unless nobody else gets to it first).

please do

gh-81322: support multiple separators in StreamReader.readuntil (#16429)

Thanks again!

There's one detail in a different repo that you may want to get ahead of. There's a declaration for this method in typeshed; ideally we should update this conditionally with a check for sys.version_info.

Thanks again!

There's one detail in a different repo that you may want to get ahead of. There's a declaration for this method in typeshed; ideally we should update this conditionally with a check for sys.version_info.

Thanks for the reminder - I'd meant to ask about typeshed but forgot. I'll sort out a PR there when I get a chance.

@gvanrossum

Looking at the typeshed entry for readuntil is giving me second thoughts about this PR. In typeshed, it's written like this:

    # Can be any buffer that supports len(); consider changing to a Protocol if PEP 688 is accepted
    async def readuntil(self, separator: bytes | bytearray | memoryview = b"\n") -> bytes: ...

On the other hand, in this PR the new use case is detected like this:

        if isinstance(separator, bytes):
            separator = [separator]
        else:
            # Makes sure shortest matches wins, and supports arbitrary iterables
            separator = sorted(separator, key=len)

So I think this PR will actually break the existing use case of passing a bytearray (or memoryview), because it would be incorrectly interpreted as an iterable of separators rather than a single separator.

@gvanrossum any thoughts on how to better decide whether we've been passed a single separator or an iterable of them? Perhaps instead of isinstance(separator, bytes), one could test ~~hasattr(separator, "__buffer__")~~ isinstance(separator, collections.abc.Buffer)? Or would it be better to introduce a new parameter (or new method) to make the user tell us the intention instead of guessing?

Fix Stream.readuntil with non-bytes buffer objects

I've opened #117653 to use collections.abc.Buffer instead of bytes to decide the behaviour.

@gvanrossum

@gvanrossum any thoughts on how to better decide whether we've been passed a single separator or an iterable of them? Perhaps instead of isinstance(separator, bytes), one could test ~~hasattr(separator, "__buffer__")~~ isinstance(separator, collections.abc.Buffer)? Or would it be better to introduce a new parameter (or new method) to make the user tell us the intention instead of guessing?

Well, my intuition would be to only allow tuples -- this follows the lead of e.g. str.startswith():

>>> "abc".startswith("a")
True
>>> "abc".startswith(("a", "b"))
True
>>> "abc".startswith(["a", "b"])
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
    "abc".startswith(["a", "b"])
    ~~~~~~~~~~~~~~~~^^^^^^^^^^^^
TypeError: startswith first arg must be str or a tuple of str, not list
>>>

Well, my intuition would be to only allow tuples -- this follows the lead of e.g. str.startswith():

I can see the appeal in that. It's also consistent with isinstance which takes tuples and not lists.

If someone does pass a list, they're probably going to get a cryptic error message; so it would be nice if we could detect incorrect usage. So maybe if it's not a tuple AND it's not a collections.abc.Buffer, raise a helpful TypeError?

If someone does pass a list, they're probably going to get a cryptic error message; so it would be nice if we could detect incorrect usage. So maybe if it's not a tuple AND it's not a collections.abc.Buffer, raise a helpful TypeError?

Maybe, though I worry about overthinking this. If they pass something incorrect they already get a cryptic error message (e.g. what if they pass a string?). Do we care? Probably not, as long as it doesn't silently do the wrong thing.

I've created a patch to accept only tuples in #117723.

Add pythongh-81322 to Doc/whatsnew/3.13.rst

There's one detail in a different repo that you may want to get ahead of. There's a declaration for this method in typeshed; ideally we should update this conditionally with a check for sys.version_info.

I've opened python/typeshed#11755

pythongh-81322: support multiple separators in StreamReader.readuntil (…

Navigation Menu

Repository navigation

Allow multiple separators in Stream.readuntil #81322

12 remaining items

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Allow multiple separators in Stream.readuntil #81322

Description

Activity

bmerry commented on Jun 3, 2019

asvetlov commented on Jun 3, 2019

bmerry commented on Jun 3, 2019

asvetlov commented on Jun 3, 2019

bmerry commented on Jun 3, 2019

asvetlov commented on Jun 3, 2019

bmerry commented on Jun 3, 2019

bmerry commented on Sep 12, 2019

asvetlov commented on Sep 12, 2019

12 remaining items

gvanrossum commented on Apr 8, 2024

bmerry commented on Apr 8, 2024

bmerry commented on Apr 8, 2024

bmerry commented on Apr 8, 2024

gvanrossum commented on Apr 8, 2024

bmerry commented on Apr 8, 2024

gvanrossum commented on Apr 8, 2024

bmerry commented on Apr 10, 2024

bmerry commented on Apr 14, 2024

Metadata

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Issue actions