-
Notifications
You must be signed in to change notification settings - Fork 48
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
(PXP-10172): Fix invalid sequence warning #1056
base: master
Are you sure you want to change the base?
Conversation
Pull Request Test Coverage Report for Build 13314
💛 - Coveralls |
tests/dbgap_sync/test_user_sync.py
Outdated
@@ -717,7 +717,7 @@ def mock_merge(dbgap_servers, sess): | |||
|
|||
# this function will be called once for each sftp server | |||
# the test config file has 3 dbgap sftp servers |
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.
guessing this changed to 2?
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.
why is this change in the tests needed? the code change shouldn't have any impact
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.
Lol i dont remember making this change. Changing this back because its obviously breaking the unit tests
@@ -370,7 +369,7 @@ def _match_pattern(filepath, id_patterns, encrypted=True): | |||
Returns: | |||
bool: whether the pattern matches | |||
""" | |||
id_patterns.append("authentication_file_phs(\d{6}).(csv|txt)") | |||
id_patterns.append("authentication_file_phs(\\\d{6}).(csv|txt)") |
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.
doesn't this mean that you expect an actual \
in the file name?
i fixed the same warning in other repos last week, you can use a raw string to address this warning: r"..."
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.
Yeah i thought that was the solution too but it doesn't work :/
Bug Fixes
DeprecationWarning: invalid escape sequence '\d'
in usersync while trying to read whitelist filename patterns.