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

default kwargs are evaluated at definition time #29

Closed
wants to merge 1 commit into from

Conversation

stavxyz
Copy link
Contributor

@stavxyz stavxyz commented Jul 10, 2014

After seeing some strange behavior while trying to write unittests for my script which uses python-whois, I thought I would go ahead and submit this patch. This PR addresses a comment made here:

d86e4ba#commitcomment-6967148

This stackoverflow question covers the topic pretty well:
“Least Astonishment” in Python: The Mutable Default Argument

From docs.python.org:

Default parameter values are evaluated when the function definition is
executed. This means that the expression is evaluated once, when the
function is defined, and that the same “pre-computed” value is used for
each call. This is especially important to understand when a default
parameter is a mutable object, such as a list or a dictionary: if the
function modifies the object (e.g. by appending an item to a list), the
default value is in effect modified. This is generally not what was
intended. A way around this is to use None as the default, and
explicitly test for it in the body of the function

https://docs.python.org/2/reference/compound_stmts.html#function-definitions

def get_whois_raw(domain, server="", previous=[], rfc3490=True, never_cut=False, with_server_list=False, server_list=[]):
def get_whois_raw(domain, server="", previous=None, rfc3490=True, never_cut=False, with_server_list=False, server_list=None):
previous = previous or []
server_list = server_list or []
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

>>> None or []
[]

@joepie91
Copy link
Owner

Thanks, that should solve the problem :) Precomputed default values is not quite the behaviour one would expect from a developer POV...

I did notice, though, that your commit introduces some mixed spacing. Could you replace the spaces in your commit with tabs?

@joepie91 joepie91 self-assigned this Jul 11, 2014
def get_whois_raw(domain, server="", previous=[], rfc3490=True, never_cut=False, with_server_list=False, server_list=[]):
def get_whois_raw(domain, server="", previous=None, rfc3490=True, never_cut=False, with_server_list=False, server_list=None):
previous = previous or []
server_list = server_list or []
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

>>> None or []
[]

@joepie91
Copy link
Owner

Thanks, but there seems to be one more case of mixed indentation in your current commit: https://github.com/joepie91/python-whois/pull/29/files#diff-c46610dab7ed75c023fafd0219efb40bR999 (line 999)

As for PEP8; I am aware that it recommends spaces, however:

  1. The core purpose of the PEP8 recommendation is standardization of the standard library, not all Python code in existence.
  2. There is a very simple case to be made for using tabs rather than spaces: it allows for a developer to set their own preferred indentation display width, without actually affecting the file - something that is especially important for open-source projects, as there will often be a wide variety of developers working on it, on a wide variety of devices and screen sizes. The implementation for tab width modification in an editor is much simpler than for on-the-fly space collapsing, which means better editor support.
  3. There are no drawbacks to using tabs when they're used consistently.

The choice to use tabs was made consciously :)

This stackoverflow question covers the topic pretty well:

    http://stackoverflow.com/q/1132941/1547030

From docs.python.org:

    Default parameter values are evaluated when the function definition is
    executed. This means that the expression is evaluated once, when the
    function is defined, and that the same “pre-computed” value is used for
    each call. This is especially important to understand when a default
    parameter is a mutable object, such as a list or a dictionary: if the
    function modifies the object (e.g. by appending an item to a list), the
    default value is in effect modified. This is generally not what was
    intended. A way around this is to use None as the default, and
    explicitly test for it in the body of the function
@stavxyz
Copy link
Contributor Author

stavxyz commented Jul 21, 2014

@joepie91 Thanks for the clarification. I think the mixed tabs/spaces are finally all fixed.

@joepie91
Copy link
Owner

Looks good to me. I'll merge this manually, as the pull request appears to be targeted at master rather than at develop.

@joepie91
Copy link
Owner

I've manually merged your pull request, and the changes are included in the new 2.4.2 release I just published on PyPI. You can update using pip install --upgrade pythonwhois. Thanks for contributing!

@joepie91 joepie91 closed this Jul 21, 2014
Sign up for free to 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.

None yet

2 participants