Fix Python3 compatibility issues. #14
Conversation
python/lsst/db/utils.py
Outdated
| if not hasattr(script, 'read'): | ||
| script = open(script) | ||
| cleanup = script.close | ||
|
|
||
| # make tmpfile to store credentials and options | ||
| fd, fname = tempfile.mkstemp(text=True) |
There was a problem hiding this comment.
I am confused. If you are opening this file in text mode, why do you need to write bytes to it below?
There was a problem hiding this comment.
It's os.write(), it only accepts bytes.
There was a problem hiding this comment.
So why open it in text mode at all? Why is os.write being used rather than print?
There was a problem hiding this comment.
Because it contains text? This should only matter for newline chars, I guess, not encoding. mkstemp() return OS-level file descriptor which cannot be used with print.
There was a problem hiding this comment.
Ok. You win. I think the tail is wagging the dog a bit here. I'm not sure I understand why a special low level file descriptor is needed here rather than a boring Python-level file handle. There is no comment in the code explaining why that's necessary and it looks like the file is opened, text is written and the file is closed.
There was a problem hiding this comment.
Well, I need both file (descriptor) and a name, and I don't care much if it is a descriptor or a true file object. I could switch to tempfile.NamedTemporaryFile if that makes things clearer.
There was a problem hiding this comment.
OK, should be better now
Fix for unicode/bytes when writing via low-level os.write call. Suppress warning from ConfigParser which has deprecated one of the methods in python3. Replace low-level file descriptor operations with file-like object when writing options to a temporary file.
Fix for unicode/bytes when writing via low-level os.write call. Suppress
warning from ConfigParser which has deprecated one of the methods in
python3.