Port to Python3|TelepathyGlib #21

Saumya-Mishra9129 · 2020-03-22T06:43:02Z

@quozl I have tested the activity it is working properly . The shell.log contains Normal Successfull completion, pid 1026 activity_id 7d2d4f14a8afe2e95921ba35e602ec40c6bfc7a2`. Kindly review it.

quozl

Thanks.

How did you test collaboration?

Can you work toward making sprites.py consistent with sprites.py in other activities that have been ported? There's no reason to make this one different, and some good reasons to make it the same.

SearchActivity.py

game.py

ayushnawal · 2020-03-22T23:37:42Z

Thanks @Saumya-Mishra9129

You can have a look at deducto and review the changes made by @chimosky in sprites.py.

As @quozl pointed out, you have used old_div, in spite of this you could have simply used //.
Although both give the same result // is what we used while porting an activity from python 2 to 3.

You can test collaboration by setting up two sugar environments and confirm that you can collaborate

Also, there is already a pull request #20 with the title Port to Python 3, I have not reviewed that completely but some changes were similar, I hope there will be no conflict when @quozl merges.

quozl · 2020-03-23T00:21:24Z

Thanks for comments.

As @quozl pointed out, you have used old_div, in spite of this you could have simply used //.
Although both give the same result // is what we used while porting an activity from python 2 to 3.

old_div is for writing code compatible with both Python 2 and Python 3. Only the Sugar Toolkit is required to do that. Activities are mostly either Python 2 or Python 3.

It is best to test to find the type of the arguments given to the division. Use / if one or more of the arguments were floating point when tested on Python 2. Use // if there's a need to avoid a floating point result on Python 3. Reminds us again why you must be able to test the activity on Python 2 before porting.

Also, there is already a pull request #20 with the title Port to Python 3, I have not reviewed that completely but some changes were similar, I hope there will be no conflict when @quozl merges.

It is nice to have more than one person working on a problem, but I do expect more cooperation and sharing of the patches. Unless patches are shared between the pull requests, then whichever pull request is ready to merge will merge first, then the other pull request will be considered as additional and may need to rebase. That's why it is very helpful for the two developers who began the two pull requests to review and merge each other's work into their own branches, rather than try to work apart.

Saumya-Mishra9129 · 2020-03-23T12:20:40Z

I will take care of cooperation next time. Is there any other activity rather than Sugar Toolkit which needs compatibility with both python 2 and 3? I was using old_div so that division operator work fine with python3 as with python2. I was using futurize as a tool for conversion from python 2 to python 3. I guess I should use 2to3 tool only.

ayushnawal · 2020-03-23T18:53:02Z

Is there any other activity rather than Sugar Toolkit which needs compatibility with both python 2 and 3?

In my opinion, No. Sugar Toolkit needs compatibility with both python 2 and 3 because some activities are running on python3 and some are still on python2( like this activity). So in order to support both the type of activities, the sugar toolkit is made compatible for both 2 and 3.

I was using futurize as a tool for conversion from python 2 to python 3

I don't know much about futurize and it's advantages over 2to3 tool but the porting guide suggests to use 2to3.

Have a look at Porting Guide (in case you haven't 😁 )

Thanks

quozl · 2020-03-24T03:10:10Z

I will take care of cooperation next time. Is there any other activity rather than Sugar Toolkit which needs compatibility with both python 2 and 3?

No, not that I know of.

I did consider it when planning the port for all of Sugar, because I'm to maintain Python 2 branches for use on the OLPC XO laptops which are stuck on Python 2 (not your problem). But the amount of change in activities has not been sufficient to justify the effort. Instead I backport patches as needed from master to python2 branch.

quozl · 2020-03-24T20:46:35Z

Don't want to be that dude, but the // operator works the same way on Python 2 and Python 3.

Well said, Sanjay, thanks. (Aside; a trouble with discussing style and porting in pull requests is that not everyone who has an opinion is present. Best in future if challenges to our style are discussed on sugar-devel@).

Saumya-Mishra9129 · 2020-03-24T21:51:00Z

@quozl @sanjay237 thanks for suggestions.

JuiP · 2020-03-27T06:30:39Z

I was using futurize as a tool for conversion from python 2 to python 3. I guess I should use 2to3 tool only.

Almost every output of 2to3 will need modification to provide backward compatibility with Python 2. On the other hand, the python-future project provides a script called futurize that will produce code that is compatible with both Python 2 and Python 3. I hope I cleared your confusion why you should use 2to3.

Saumya-Mishra9129 · 2020-04-23T20:31:09Z

I have tried to fix Collaboration of activity as I am trying to port the activity to Collabwrapper , the port is still under process but yet I have made some changes. A review is requested here to proceed further as I am still getting some D-Bus connection related errors.

quozl · 2020-04-24T03:33:31Z

In general it looks kinda right, but you need to get it working before it is worth doing a detailed review. If you face an error you cannot resolve, do ask about it.

SearchActivity.py

chimosky · 2020-04-29T01:28:16Z

I agree with @quozl above, I took a look and made some comments.

Source - https://github.com/sugarlabs/turtleart-activity/blob/master/TurtleArt/sprites.py

add len argument in Gtk.Clipboard set_text method set len=-1 Fixes Error: TypeError: Gtk.Clipboard.set_text() takes exactly 3 arguments (2 given)

Saumya-Mishra9129 · 2020-06-22T18:51:33Z

I am making new commits here as previous commits and port was based on futurize tool , and It makes difficult in reviewing as well as it creates a lot of confusion. I am done with Port to Python3 and TelepathyGLib , and will work on fixing collaboration now.

write method here used to take unicode in python2 use encoded bytes in place of unicode in python3

Saumya-Mishra9129 · 2020-06-23T08:51:14Z

Tested Collaboration, working i.e. game state is shared. However when a player wins and goes to new level, until second player doesn't reach to same level as first, game state is not shared.
It is shared with both on same levels. I don't know what way the collaboration is desired here .
@quozl @chimosky @pro-panda Please review.

quozl · 2020-06-29T06:57:23Z

So to paraphrase, you're not sure how to implement shared game state after the first round? I think the game state should be the same; perhaps the new level should only occur once both players have won it once. If a wider discussion is needed, use the issue #18.

Saumya-Mishra9129 · 2020-06-29T07:18:15Z

you're not sure how to implement shared game state after the first round?

No that is not the case. The way I have implemented collaboration , works only when both players plays on same levels. If one player wins first, collaboration stops and resumes when second player comes on same level as first. however as you said level upgradation should only be possible when both players win. I was saying about perhaps it can be the case that author who have designed the activity , wanted collaboration this way. I am not sure, but it can be possible.

JuiP · 2020-06-29T07:52:40Z

I haven't tested collaboration, but is it possible to start a new game when either player wins? This way you can ensure that the game state will always be shared.

chimosky · 2020-06-29T23:14:39Z

SearchActivity.py

+
+        self._collab = CollabWrapper(self)
+        self._collab.connect('message', self._message_cb)
+        self._collab.connect('joined', self._joined_cb)


It'll be better to have the joined signal connected from one API to one callback, not having one from two different APIs to same callback.

SearchActivity.py

chimosky · 2020-06-29T23:24:13Z

SearchActivity.py

        self._game.remote_button_press(dot, color)

-    def send_event(self, entry):
+    def send_event(self, command, payload):


You can also get rid of this method and simply have collab.post calls where you need them.

chimosky · 2020-06-29T23:33:28Z

@Saumya-Mishra9129 said

No that is not the case. The way I have implemented collaboration , works only when both players plays on same levels. If one player wins first, collaboration stops and resumes when second player comes on same level as first. however as you said level upgradation should only be possible when both players win. I was saying about perhaps it can be the case that author who have designed the activity , wanted collaboration this way. I am not sure, but it can be possible.

Collaboration should work when players are on the same level and when one wins, the other player's game state should be automatically updated and the player should be on the same level as the previous winner.

quozl · 2020-06-29T23:39:36Z

Collaboration should work when players are on the same level and when one wins, the other player's game state should be automatically updated and the player should be on the same level as the previous winner.

I agree. Thanks for expressing it this way.

quozl requested changes Mar 22, 2020

View reviewed changes

SearchActivity.py Outdated Show resolved Hide resolved

SearchActivity.py Outdated Show resolved Hide resolved

game.py Outdated Show resolved Hide resolved

game.py Outdated Show resolved Hide resolved

quozl mentioned this pull request Mar 22, 2020

Port to Python 3 and Collabwrapper #20

Open

Saumya-Mishra9129 changed the title ~~Port to Pyrhon3|TelepathyGlib~~ Port to Python3|TelepathyGlib Mar 24, 2020

chimosky requested changes Apr 29, 2020

View reviewed changes

SearchActivity.py Outdated Show resolved Hide resolved

SearchActivity.py Outdated Show resolved Hide resolved

SearchActivity.py Outdated Show resolved Hide resolved

SearchActivity.py Show resolved Hide resolved

This comment has been minimized.

Sign in to view

Saumya-Mishra9129 added 7 commits June 22, 2020 23:37

Run 2to3

f9ff5d9

Change exec to sugar-activity3

5280ee6

Port to TelepathyGLib

c53756f

Remove Trailing semicolon in the statement

50408ce

Update sprites.py with upstream

11c1227

Source - https://github.com/sugarlabs/turtleart-activity/blob/master/TurtleArt/sprites.py

Add PangoCairo and Gdk required versions

a40cfd3

Fix Gtk Clipboard TypeError

d88651f

add len argument in Gtk.Clipboard set_text method set len=-1 Fixes Error: TypeError: Gtk.Clipboard.set_text() takes exactly 3 arguments (2 given)

Saumya-Mishra9129 force-pushed the porting branch from 5b0f80e to d88651f Compare June 22, 2020 18:48

Encode str to bytes

aeb816e

write method here used to take unicode in python2 use encoded bytes in place of unicode in python3

Saumya-Mishra9129 force-pushed the porting branch from 7cec31e to 9b742ad Compare June 23, 2020 08:46

chimosky reviewed Jun 29, 2020

View reviewed changes

Saumya-Mishra9129 force-pushed the porting branch 2 times, most recently from a5cf0cd to 6ba39c5 Compare August 4, 2020 09:14

Port to CollabWrapper

695b73f

Saumya-Mishra9129 force-pushed the porting branch from 6ba39c5 to 695b73f Compare August 4, 2020 09:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Port to Python3|TelepathyGlib #21

Port to Python3|TelepathyGlib #21

Saumya-Mishra9129 commented Mar 22, 2020

quozl left a comment

ayushnawal commented Mar 22, 2020

quozl commented Mar 23, 2020

Saumya-Mishra9129 commented Mar 23, 2020

ayushnawal commented Mar 23, 2020

quozl commented Mar 24, 2020

quozl commented Mar 24, 2020 via email

Saumya-Mishra9129 commented Mar 24, 2020

JuiP commented Mar 27, 2020

Saumya-Mishra9129 commented Apr 23, 2020

quozl commented Apr 24, 2020

chimosky commented Apr 29, 2020

This comment has been minimized.

Saumya-Mishra9129 commented Jun 22, 2020

Saumya-Mishra9129 commented Jun 23, 2020

quozl commented Jun 29, 2020

Saumya-Mishra9129 commented Jun 29, 2020

JuiP commented Jun 29, 2020

chimosky Jun 29, 2020

chimosky Jun 29, 2020

Saumya-Mishra9129 Aug 3, 2020

chimosky commented Jun 29, 2020

quozl commented Jun 29, 2020

Port to Python3|TelepathyGlib #21

Are you sure you want to change the base?

Port to Python3|TelepathyGlib #21

Conversation

Saumya-Mishra9129 commented Mar 22, 2020

quozl left a comment

Choose a reason for hiding this comment

ayushnawal commented Mar 22, 2020

quozl commented Mar 23, 2020

Saumya-Mishra9129 commented Mar 23, 2020

ayushnawal commented Mar 23, 2020

quozl commented Mar 24, 2020

quozl commented Mar 24, 2020 via email

Saumya-Mishra9129 commented Mar 24, 2020

JuiP commented Mar 27, 2020

Saumya-Mishra9129 commented Apr 23, 2020

quozl commented Apr 24, 2020

chimosky commented Apr 29, 2020

This comment has been minimized.

Saumya-Mishra9129 commented Jun 22, 2020

Saumya-Mishra9129 commented Jun 23, 2020

quozl commented Jun 29, 2020

Saumya-Mishra9129 commented Jun 29, 2020

JuiP commented Jun 29, 2020

chimosky Jun 29, 2020

Choose a reason for hiding this comment

chimosky Jun 29, 2020

Choose a reason for hiding this comment

Saumya-Mishra9129 Aug 3, 2020

Choose a reason for hiding this comment

chimosky commented Jun 29, 2020

quozl commented Jun 29, 2020