google / python-adb

Python ADB + Fastboot implementation
Apache License 2.0
1.79k stars 357 forks source link

Fix using file instead of io.IOBase #140

Open martin2250 opened 5 years ago

martin2250 commented 5 years ago

As mentioned in #126

googlebot commented 5 years ago

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

:memo: Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here (e.g. I signed it!) and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers
martin2250 commented 5 years ago

I signed it!

googlebot commented 5 years ago

CLAs look good, thanks!

fahhem commented 5 years ago

This is not backwards-compatible with Python 2. Please check against file if available. Some options are mentioned here: https://stackoverflow.com/questions/25738602/how-to-judge-a-file-type-both-work-in-python-2-and-python-3

martin2250 commented 5 years ago

Which method do you think is better in this case, using the tuple and try/except or checking hasattr(f, 'fileno') / hasattr(f, 'write')?

fahhem commented 5 years ago

I think fileno() is a good proxy because it supports file-like objects better (like sockets, etc)

coveralls commented 5 years ago

Coverage Status

Coverage increased (+0.3%) to 43.893% when pulling 2ead78a44ca7fe4aab33afd34918451a87d48679 on martin2250:master into d9b94b2dda555c14674c19806debb8449c0e9652 on google:master.

martin2250 commented 5 years ago

Ok, the PR should be good now