Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

readinto is not a method on io.TextIOBase #80029

Closed
steverpalmer mannequin opened this issue Jan 29, 2019 · 10 comments
Closed

readinto is not a method on io.TextIOBase #80029

steverpalmer mannequin opened this issue Jan 29, 2019 · 10 comments
Labels
3.8 only security fixes docs Documentation in the Doc dir type-bug An unexpected behavior, bug, or error

Comments

@steverpalmer
Copy link
Mannequin

steverpalmer mannequin commented Jan 29, 2019

BPO 35848
Nosy @benjaminp, @vadmium, @csabella, @miss-islington, @remilapeyre, @tirkarthi, @steverpalmer
PRs
  • bpo-35848: Move all documentation regarding the readinto #11893
  • [3.7] closes bpo-35848: Move all documentation regarding the readinto out of IOBase. (GH-11893) #12736
  • 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 = <Date 2019-04-09.04:35:30.169>
    created_at = <Date 2019-01-29.09:18:40.868>
    labels = ['type-bug', '3.8', 'docs']
    title = 'readinto is not a method on io.TextIOBase'
    updated_at = <Date 2019-04-09.04:57:42.174>
    user = 'https://github.com/steverpalmer'

    bugs.python.org fields:

    activity = <Date 2019-04-09.04:57:42.174>
    actor = 'miss-islington'
    assignee = 'docs@python'
    closed = True
    closed_date = <Date 2019-04-09.04:35:30.169>
    closer = 'benjamin.peterson'
    components = ['Documentation']
    creation = <Date 2019-01-29.09:18:40.868>
    creator = 'steverpalmer'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 35848
    keywords = ['patch']
    message_count = 10.0
    messages = ['334507', '334524', '334526', '334530', '334533', '334548', '334549', '335571', '339697', '339698']
    nosy_count = 8.0
    nosy_names = ['benjamin.peterson', 'stutzbach', 'martin.panter', 'cheryl.sabella', 'miss-islington', 'remi.lapeyre', 'xtreak', 'steverpalmer']
    pr_nums = ['11893', '12736']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue35848'
    versions = ['Python 3.8']

    @steverpalmer
    Copy link
    Mannequin Author

    steverpalmer mannequin commented Jan 29, 2019

    class io.IOBase states "Even though IOBase does not declare read(), readinto(), or write() because their signatures will vary, implementations and clients should consider those methods part of the interface. Also, implementations may raise a ValueError (or UnsupportedOperation) when operations they do not support are called." However, even though class io.TextIOBase is described as inheriting from io.IOBase, a call to readinto method returns AttributeError exception indicating no readinto attribute, inconsistent with the documentation.

    @steverpalmer steverpalmer mannequin added topic-IO 3.7 (EOL) end of life type-bug An unexpected behavior, bug, or error labels Jan 29, 2019
    @SilentGhost SilentGhost mannequin added docs Documentation in the Doc dir 3.8 only security fixes and removed topic-IO labels Jan 29, 2019
    @SilentGhost SilentGhost mannequin assigned docspython Jan 29, 2019
    @tirkarthi
    Copy link
    Member

    https://docs.python.org/3/library/io.html#io.TextIOBase

    Base class for text streams. This class provides a character and line based interface to stream I/O. There is no readinto() method because Python’s character strings are immutable. It inherits IOBase. There is no public constructor.

    io.TextIOBase docs say there is no readinto method in the documentation.

    @remilapeyre
    Copy link
    Mannequin

    remilapeyre mannequin commented Jan 29, 2019

    I checked and io.TextIOBase is the only io.IOBase subclass to lack one of read, readinto or write:

    >>> import io, inspect
    >>> for name, obj in inspect.getmembers(io, predicate=inspect.isclass):
    ... 	missing = {'read', 'readinto', 'write'} - {name for name, _ in inspect.getmembers(obj)}
    ...	if issubclass(obj, io.IOBase) and missing:
    ...	     print(obj, missing, issubclass(obj, io.TextIOBase))

    <class 'io.IOBase'> {'write', 'read', 'readinto'} False
    <class '_io.StringIO'> {'readinto'} True
    <class 'io.TextIOBase'> {'readinto'} True
    <class '_io.TextIOWrapper'> {'readinto'} True

    I can open a PR to fix the conflicts between the two parts of the documentation. I think it's appropriate to change TextIOBase to raise UnsupportedOperation when calling readinto and to change the documentation accordingly.

    @remilapeyre remilapeyre mannequin added topic-IO 3.8 only security fixes and removed docs Documentation in the Doc dir 3.8 only security fixes 3.7 (EOL) end of life labels Jan 29, 2019
    @steverpalmer
    Copy link
    Mannequin Author

    steverpalmer mannequin commented Jan 29, 2019

    I agree with Karthikeyan that the method does not apply in the io.TextIOBase class context. I'm sorry that I didn't spot the note in the description of io.TextIOBase - though I think that it is easy to miss.

    I'd suggest that there are two ways to clear this up:

    1. change only the documentation to read "Even though IOBase does not declare read() or write() because their signatures will vary, implementations and clients should consider those methods part of the interface." (deleting reference to readinto())

    2. change the standard library for io.TextIOBase to add a method readinto which will raise an UnsupportedOperation.

    With option 1, the descriptions for io.RawIOBase and io.BufferedIOBase both include description of the readinto method, so nothing is lost by removing mention of it at the io.IOBase level of the hierarchy. In any case, readinto() is not defined on the io.IOBase class.

    >>> 'readinto' not in dir(io.IOBase)
    True

    With option 2, it feels like this is closer to the design intent of a common interface over similar but distinguished classes. It also avoids removing things from the documentation in case someone already has some expectations of the behaviour.

    @tirkarthi
    Copy link
    Member

    Thanks Steve for the details. I am adding io module maintainers to the issue who will have better context on whether to clarify the docs or to change the implementation to raise UnsupportedOperation.

    @vadmium
    Copy link
    Member

    vadmium commented Jan 30, 2019

    I think it would be more practical to fix the documentation (option 1). Do you have a use case for “TextIOBase.readinto” raising ValueError (something more concrete than someone having expectations)?

    @steverpalmer
    Copy link
    Mannequin Author

    steverpalmer mannequin commented Jan 30, 2019

    I don't have a "real" use case. I discovered the issue when I was developing a unittest suite for what it means to be "file-like". I've been codifying the description in the standard library and exercising my tests against the built-in file-likes, such as the io.StringIO class, when it raised the Attribute Exception.

    The more I think about it, the more like a documentation problem it feels. For example, the statement "... because their signatures will vary ..." does not apply to readinto in the cases where it is defined.

    For completeness, the note in io.TextIOBase stating "There is no readinto() method because Python’s character strings are immutable." would also need to be removed as part of a documentation fix.

    (It's also nice when solutions result in less "stuff". :-)

    @csabella
    Copy link
    Contributor

    Steve,

    Would you be interested in creating a Github pull request with the documentation changes?

    @csabella csabella added docs Documentation in the Doc dir and removed topic-IO labels Feb 14, 2019
    @benjaminp
    Copy link
    Contributor

    New changeset 7b97ab3 by Benjamin Peterson (Steve Palmer) in branch 'master':
    closes bpo-35848: Move all documentation regarding the readinto out of IOBase. (GH-11893)
    7b97ab3

    @miss-islington
    Copy link
    Contributor

    New changeset 0a16bb1 by Miss Islington (bot) in branch '3.7':
    closes bpo-35848: Move all documentation regarding the readinto out of IOBase. (GH-11893)
    0a16bb1

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.8 only security fixes docs Documentation in the Doc dir type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    5 participants