Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Port to Python3 and other fixes [GCI] #7

Merged
merged 7 commits into from Dec 24, 2019
Merged

Conversation

@srevinsaju
Copy link
Contributor

srevinsaju commented Dec 19, 2019

The code has been heavily changed, The list_set union doesn't work on python3 due to change in override methods in classes. Tested on SugarDE. Linting done on most files

srevinsaju added 5 commits Dec 19, 2019
dobject.py Outdated Show resolved Hide resolved
dobject.py Outdated Show resolved Hide resolved
dobject.py Outdated Show resolved Hide resolved
…et* functions, excluding variabole names
@srevinsaju

This comment has been minimized.

Copy link
Contributor Author

srevinsaju commented Dec 20, 2019

@quozl , fixed it on 0b3a93d

@quozl

This comment has been minimized.

Copy link
Contributor

quozl commented Dec 22, 2019

Thanks. Reviewed again. Can you explain why self._set is changed to self._list in dobject? There is so much about dobject that is based on the mathematical concept of sets, and you've changed a key variable name but I can't see why. I've no trouble with self._set being a list datatype. Why did it have to change?

My hesitation also stems from having to maintain a python2 branch into the future, and the amount of change in dobject.py is huge, for this reason and also whitespace.

@srevinsaju

This comment has been minimized.

Copy link
Contributor Author

srevinsaju commented Dec 22, 2019

Yes @quozl , I have changed from set to list due to the following reasons

  • The existing class ListSet uses a lot of deprecated method like cmp and other lambda functions which is not compatible with Python3
  • A new number of methods were added to the ListSet to give it the settish properties of having unique values, so on and so forth
  • Changing from set to list, helps in keeping the source class as a base of list type than set. List with set properties gives the class ListSet .
    ListSet = builtin list + additional set functions
    Which is easier to maintain on python3 than
    ListSet = builtin set + additional list functions
    Hope it is intuitive for you. It was the only workaround to the problem i faced. Sets and Lists can be natively methodized to use it under the sorted function. However not the ListSet class inherited from Javascript

PS: One more issue, If i continue using _set, it raises an error.
self._list is not found
Because the dobject calls self._list.

@quozl

This comment has been minimized.

Copy link
Contributor

quozl commented Dec 23, 2019

Thanks.

@bemasc, this is a bit beyond my pay grade, and I'm worried I may accept regressions; have you time to review?

@srevinsaju

This comment has been minimized.

Copy link
Contributor Author

srevinsaju commented Dec 23, 2019

@quozl, is it necessary to maintain a python2 branch? python 2 is at the verge of its death and new versions of GNU are not going to include python2 supposedly, idk. and the earlier made releases would be enough for the older sugar xo. it might be a part of your oversight plan, and I would like to know more on that :)

@quozl

This comment has been minimized.

Copy link
Contributor

quozl commented Dec 23, 2019

@srevinsaju, thanks for asking, yes, it is one of the things I'm paid to do by OLPC. We made about three million laptops which have Sugar and Python 2. We can't practically upgrade to Python 3 due to cost. Cost of software engineering for firmware, kernel, drivers, configuration, and then operating system building. For most of the activities I focus on, I do maintain master, python2 and also fedora18 branches, and try to port useful patches from master into the other branches. Minimising change helps with that. For instance, your whitespace changes are good, and I'd take them, though it does mean I'll have to pick the changes out of your patch. I'm fine with doing that.

This reverts parts of commit f185cb7.
@quozl quozl force-pushed the srevinsaju:python3-ss branch from 23c0f32 to db8f54c Dec 24, 2019
@quozl quozl merged commit 209e566 into sugarlabs:master Dec 24, 2019
Copy link
Contributor

bemasc left a comment

It's been many years since I looked at this, so I don't have any special insight, but thanks for maintaining it. The GObject and _thread changes look a little scary but probably fine.

@@ -498,7 +498,7 @@ def _highscore_cb(self, val, score):
for L in self._listeners:
L(val)

class AddOnlySet:
class AddOnlySet(ListSet):

This comment has been minimized.

Copy link
@bemasc

bemasc Jan 2, 2020

Contributor

Was there a reason for this change to inherit from ListSet? It seems like a good idea, but this probably makes all of the copying of operator methods (like self.__and__) unnecessary.

@quozl

This comment has been minimized.

Copy link
Contributor

quozl commented Jan 3, 2020

@bemasc, thanks.

@srevinsaju, I know you don't have resources to test collaboration, but I think testing collaboration is important, because that's what the GroupThink component is for. We will hvae to wait for someone to do it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.