-
Notifications
You must be signed in to change notification settings - Fork 57
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
Quote exclusion regexes properly (#113) #114
Quote exclusion regexes properly (#113) #114
Conversation
* Regex-quote arbitrary files added to exclusion list * Add tests around this behaviour demonstrating regex nature * Update examples and docs to demonstrate this * Use immutable keyword parameters defaults too
@@ -223,6 +224,7 @@ def package(self, ignore=[]): | |||
those files when creating the zip file. The paths to be matched are | |||
local to the source root. | |||
""" | |||
ignore = ignore or [] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand why this was added in favor over setting the default for the ignore arg to [].
@@ -21,7 +21,8 @@ | |||
LOG = logging.getLogger(__name__) | |||
|
|||
|
|||
def copy_tree(src, dest, ignore=[], include_parent=False): | |||
def copy_tree(src, dest, ignore=None, include_parent=False): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Again, this seems like an extra step that the default arg already covers.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you give us a summary of why it is you updated the default arg for ignore to be None rather than [].
Sure: mutable default arguments can often be a source bugs in Python, and some IDEs warn about their usage. This book / guide has the best explanation / example I've seen. |
I've never run into that. Thanks! |
Thank you |
Fixes #113