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

mock.patch could whitelist builtins to not need create=True #61860

Closed
voidspace opened this issue Apr 8, 2013 · 11 comments
Closed

mock.patch could whitelist builtins to not need create=True #61860

voidspace opened this issue Apr 8, 2013 · 11 comments
Assignees
Labels
easy stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@voidspace
Copy link
Contributor

voidspace commented Apr 8, 2013

BPO 17660
Nosy @pitrou, @merwok, @voidspace, @berkerpeksag, @kushaldas
Files
  • issue17660.patch: initial patch
  • issue17660_v3.patch: Updated patch with builtins module.
  • 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 = 'https://github.com/voidspace'
    closed_at = <Date 2014-04-14.20:25:31.121>
    created_at = <Date 2013-04-08.10:48:09.957>
    labels = ['easy', 'type-bug', 'library']
    title = 'mock.patch could whitelist builtins to not need create=True'
    updated_at = <Date 2014-04-15.19:28:37.514>
    user = 'https://github.com/voidspace'

    bugs.python.org fields:

    activity = <Date 2014-04-15.19:28:37.514>
    actor = 'eric.araujo'
    assignee = 'michael.foord'
    closed = True
    closed_date = <Date 2014-04-14.20:25:31.121>
    closer = 'python-dev'
    components = ['Library (Lib)']
    creation = <Date 2013-04-08.10:48:09.957>
    creator = 'michael.foord'
    dependencies = []
    files = ['29731', '34825']
    hgrepos = []
    issue_num = 17660
    keywords = ['patch', 'easy']
    message_count = 11.0
    messages = ['186290', '186291', '186301', '186323', '186347', '215917', '216113', '216201', '216360', '216364', '216366']
    nosy_count = 6.0
    nosy_names = ['pitrou', 'eric.araujo', 'michael.foord', 'python-dev', 'berker.peksag', 'kushal.das']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue17660'
    versions = ['Python 3.5']

    @voidspace
    Copy link
    Contributor Author

    voidspace commented Apr 8, 2013

    When patching builtin names (e.g. open) in a specific namespace you need to specify "create=True" or patch will refuse to create a name that doesn't exist.

    patch could whitelist the builtin names, when the patch target is a module object, to not require the "create=True".

    @voidspace voidspace self-assigned this Apr 8, 2013
    @voidspace voidspace added stdlib Python modules in the Lib dir easy type-bug An unexpected behavior, bug, or error labels Apr 8, 2013
    @kushaldas
    Copy link
    Member

    kushaldas commented Apr 8, 2013

    Working on this.

    @kushaldas
    Copy link
    Member

    kushaldas commented Apr 8, 2013

    Initial patchset along with documentation and tests update.

    @pitrou
    Copy link
    Member

    pitrou commented Apr 8, 2013

    To be honest this proposal sounds like a quirk more than a feature to me. If you only special-case builtins, people will have to remember that special case and it will make the API more complicated.

    @voidspace
    Copy link
    Contributor Author

    voidspace commented Apr 8, 2013

    I don't think that's a particular issue. In general you only need to use "create=True" if a name is *not* available in a namespace.

    Builtin names are odd in that you can use them in a namespace even though they don't exist there - so you have to *remember* to use "create=True" even though the name *is* available.

    So this issue is about fixing that quirk.

    @merwok
    Copy link
    Member

    merwok commented Apr 11, 2014

    Reviewed on Rietveld.

    @kushaldas
    Copy link
    Member

    kushaldas commented Apr 14, 2014

    Updated patch with builtins module.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Apr 14, 2014

    New changeset e457de60028c by Michael Foord in branch 'default':
    Closes bpo-17660. You no longer need to explicitly pass create=True when patching builtin names.
    http://hg.python.org/cpython/rev/e457de60028c

    @python-dev python-dev mannequin closed this as completed Apr 14, 2014
    @berkerpeksag
    Copy link
    Member

    berkerpeksag commented Apr 15, 2014

    + .. note::

    I think using a note directive is not necessary here. Also it looks a bit ugly :) https://dl.dropboxusercontent.com/u/166024/issue17660.png

    •    .. versionchanged:: 3.5
      
    •       If you are patching builtins in a module then you don't
      
    •       need to pass `create=True`, it will be added by default.
      

    @voidspace
    Copy link
    Contributor Author

    voidspace commented Apr 15, 2014

    Personally I don't think it looks ugly and that it is a point worth calling out. Other opinions welcomed.

    @merwok
    Copy link
    Member

    merwok commented Apr 15, 2014

    I noticed this in the commit; I don’t think the rendered output is really ugly, but the markup does look strange, as it’s two nested note(-like) directives.

    @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
    easy stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    5 participants