Arvados

Overall I thought the changes made sense and increased readability. One question I had was about file

sdk/python/arvados/_normalize_stream.py

path = re.sub('\\\\', lambda m: '\\134', path)
path = re.sub('[:\000-\040]', lambda m: "\\%03o" % ord(m.group(0)), path)

with the single line:

re.sub(r'[\\:\000-\040]', lambda m: "\\%03o" % ord(m.group(0)), path)

so the logic of the first regex is no longer present. I just wanted to double check that removing that logic didn't have any unintended impacts.

I looked at commit 6daa187dccdc8d7513417d5122fd145661647617.

Alex Coleman wrote in #note-4:

the logic of the first regex is no longer present. I just wanted to double check that removing that logic didn't have any unintended impacts.

I think this is probably the hairiest part of the diff, but the logic hasn't been removed, it's just been merged into a single call.

In the pattern, \\ has been added to the character group, so it now matches backslashes as the original first regexp did.

And when you call the replacement function with a backslash match, it returns the same result that was hardcoded in the original:

>>> '\\%03o' % ord('\\')
'\\134'

So the results are the same, we just get them with one regexp instead of two.

Thanks, that makes sense. I just wanted to double-check. In that case, it looks good to me to merge.