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

Allow setting line_buffering on existing TextIOWrapper #74711

Closed
pitrou opened this issue May 31, 2017 · 8 comments
Closed

Allow setting line_buffering on existing TextIOWrapper #74711

pitrou opened this issue May 31, 2017 · 8 comments
Labels
3.7 expert-IO type-feature

Comments

@pitrou
Copy link
Member

@pitrou pitrou commented May 31, 2017

BPO 30526
Nosy @ncoghlan, @pitrou, @benjaminp, @serhiy-storchaka
PRs
  • #1887
  • #1922
  • 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 2017-06-03.15:48:47.209>
    created_at = <Date 2017-05-31.16:15:46.650>
    labels = ['type-feature', '3.7', 'expert-IO']
    title = 'Allow setting line_buffering on existing TextIOWrapper'
    updated_at = <Date 2017-06-03.15:48:47.208>
    user = 'https://github.com/pitrou'

    bugs.python.org fields:

    activity = <Date 2017-06-03.15:48:47.208>
    actor = 'pitrou'
    assignee = 'none'
    closed = True
    closed_date = <Date 2017-06-03.15:48:47.209>
    closer = 'pitrou'
    components = ['IO']
    creation = <Date 2017-05-31.16:15:46.650>
    creator = 'pitrou'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 30526
    keywords = []
    message_count = 8.0
    messages = ['294850', '294922', '294925', '294931', '294933', '295016', '295017', '295074']
    nosy_count = 5.0
    nosy_names = ['ncoghlan', 'pitrou', 'benjamin.peterson', 'stutzbach', 'serhiy.storchaka']
    pr_nums = ['1887', '1922']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue30526'
    versions = ['Python 3.7']

    @pitrou
    Copy link
    Member Author

    @pitrou pitrou commented May 31, 2017

    Currently, if you want to change the line buffering behaviour of standard streams, you can create a new stream and assign it to sys.{stdout,stderr,stdin}. Unfortunately, it is common for references to the old streams to be stored in various places (such as logging configuration, or third-party libraries). Replacing them all is probably a hopeless cause. It would be much better if one could simply write sys.stdout.line_buffering = True and be done with it.

    @pitrou pitrou added 3.7 expert-IO type-feature labels May 31, 2017
    @serhiy-storchaka
    Copy link
    Member

    @serhiy-storchaka serhiy-storchaka commented Jun 1, 2017

    This looks similar to bpo-15216 and may be merged with it. At least setting line_buffering and encoding should have unified interface. I see two possibilities of making this Pythonic:

    • Make attributes writeable.

    • Add a method (configure(), reopen(), or like) that takes new values as keyword arguments. This allows to change several attributes at a time.

    @pitrou
    Copy link
    Member Author

    @pitrou pitrou commented Jun 1, 2017

    I don't think those two issues need to be merged. Setting line_buffering and write_through is almost trivial, while setting the encoding seems to take a significant amount of effort.

    That said, I agree that a unified interface may be nice. Perhaps a reconfigure(**kwargs) method?

    @serhiy-storchaka
    Copy link
    Member

    @serhiy-storchaka serhiy-storchaka commented Jun 1, 2017

    First at all, I'm not sure that allowing to modify TextIOWrapper settings after creation is a good idea. I would be feeling uncomfortable if the third-party library changed the buffering or encoding settings of passed text stream for its own needs, and this stream is sys.stdout. For example set f.line_buffering = False for performance and break your logging. Creating a new TextIOWrapper for own needs looks safer. But changing the global state is the essential part of Nick's and your issues.

    This could have long-term consequences, so it is worth to discuss the principle on the Python-Dev mailing list before applying the changes.

    @pitrou
    Copy link
    Member Author

    @pitrou pitrou commented Jun 1, 2017

    Le 01/06/2017 à 12:41, Serhiy Storchaka a écrit :

    First at all, I'm not sure that allowing to modify TextIOWrapper settings after creation is a good idea. I would be feeling uncomfortable if the third-party library changed the buffering or encoding settings of passed text stream for its own needs, and this stream is sys.stdout.

    Agreed that third-party libraries should not, but we're talking about
    applications here.

    For example set f.line_buffering = False for performance and break your logging. Creating a new TextIOWrapper for own needs looks safer.

    Now you may have two TextIOWrappers alive wrapping the same buffered IO
    object.

    @ncoghlan
    Copy link
    Contributor

    @ncoghlan ncoghlan commented Jun 2, 2017

    Right, the request/requirement for in-place configuration changes arise from the fact the underlying buffer objects can't readily be shared, while makes swapping out the wrapper problematic if others may already have references to the original.

    The "Don't do this implicitly in a library" admonition is then really the same one that applies to any unusual tinkering with global state (e.g. monkeypatching): you can definitely break things if you do it carelessly, so let the main application decide if and when global state updates are appropriate.

    I do like "reconfigure()" as the API name, since that's then nicely amenable to expanding to all the settings supported by open() as concrete use cases arise. One thing we'll need to watch out for is the difference between "Leave this setting alone" and "Revert this setting to the global default".

    @pitrou
    Copy link
    Member Author

    @pitrou pitrou commented Jun 2, 2017

    Thanks. I'll produce an updated PR using reconfigure() for line_buffering and write_through.

    @pitrou
    Copy link
    Member Author

    @pitrou pitrou commented Jun 3, 2017

    New changeset 3c2817b by Antoine Pitrou in branch 'master':
    Fix bpo-30526: Add TextIOWrapper.reconfigure() and a TextIOWrapper.write_through attribute (bpo-1922)
    3c2817b

    @pitrou pitrou closed this as completed Jun 3, 2017
    @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.7 expert-IO type-feature
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants