Python 2 -> 3 changes. #1745
Python 2 -> 3 changes. #1745
Conversation
|
@PeterSurda in case github doesn't ping you automatically. |
|
Thanks, maybe we can use parts of it. As it is unfortunately I can't accept it as none of the test jobs succeed, it breaks compatibility with python 2.7 while still not working correctly with 3.7. You can run the tests yourself with |
This doesn't surprise me since I haven't been able to set up everything required by bitmessage under python 2 in latest fedora and cannot test compatibility of my changes. But if you're open to these contributions and no one else is working on this at the moment I can test python 2 stuff in a virtual machine so that things don't break. |
|
@iljah well, there are developers working on python3 port, it just isn't always visible here as the work may not be ready for a PR until it's done correctly and I'm happy with it. I worry perhaps you're doing duplicate work. If you want to continue development, as I said you can run the tests with the |
| from network.invthread import InvThread | ||
| from network.receivequeuethread import ReceiveQueueThread | ||
| from network.downloadthread import DownloadThread | ||
| from network.uploadthread import UploadThread |
There was a problem hiding this comment.
You are binding the code to pathmagic harder instead of preparing its removal ):
network is a package, so you can write from .network import .. ant it will work both in python2.7 and python3 in installed and portable mode.
There was a problem hiding this comment.
Thanks it wasn't very clear to me what the best way to fix imports for python 3 would be. I'll give .network a try.
|
Also, there is WIP to get PyBitmessage flatpak builds working, #1618 . This may also alleviate pressure from a python3 port and serve as a workaround for new releases of linux distros. |
| @@ -75,7 +75,7 @@ def lock(self): | |||
| fcntl.lockf(self.fp, fcntl.LOCK_EX | fcntl.LOCK_NB) | |||
| self.lockPid = os.getpid() | |||
| except IOError: | |||
| print 'Another instance of this application is already running' | |||
| print('Another instance of this application is already running') | |||
| sys.exit(-1) | |||
There was a problem hiding this comment.
sys.exit('Another instance of this application is already running')?
There was a problem hiding this comment.
Possible but in that case return value can't be specified. If nothing relies on particular return values then exit('') is probably better.
There was a problem hiding this comment.
Only two values are significant here: 0 and any other. When you pass a string it's interpreted as program error, like any non-zero exit code.
There was a problem hiding this comment.
I guess nothing relies on specific values of non-zero so exit('') is fine.
$ python -c 'exit(5)'; echo $?
5
$ python -c 'exit(6)'; echo $?
6
$ python -c 'exit("error")'; echo $?
error
1
| @@ -0,0 +1 @@ | |||
| from .storage import InventoryItem, InventoryStorage | |||
There was a problem hiding this comment.
You are adding __init__ here and remove it from network, using dotted imports here, but rely on pathmagic in other places. So inconsistent.
There was a problem hiding this comment.
I've (luckily? :)) never had to deal with this part of python so don't know what would be best/correct/portable fix. I'll try .stuff
| @@ -40,12 +40,12 @@ def translateText(context, text, n=None): | |||
| try: | |||
There was a problem hiding this comment.
There was a problem hiding this comment.
Didn't know this existed. Do you suggest starting my work from that or incorporating some changes into my fork?
There was a problem hiding this comment.
You can see all the forks on the network page: https://github.com/Bitmessage/PyBitmessage/network I suggest you to keep trying if you wanna merge, but be prepared to long wait. Existing PR's and issues are also worth reading, for example #1712
|
Created #1746 that includes fewer changes but still works using python 2. |
Didn't see much work being done lately for transition from python 2 to 3 and was wondering if something like this would be helpful. If yes I can submit request(s) that follow guidelines better...