Major rewriting of class implementation

[Pierre Quentel]

In issue #766 Glyph requested the support of the popular attrs package.

When I started working on it I realized that there was a big issue with the way classes are currently implemented.

The following code fails on the version currently on line at brython.info:

class A:

    def __init__(self):
        self.version = "old"

def new_init(obj):
    obj.version = "new"

A.__init__ = new_init

assert A().version == "new"

In the current implementation, classes are represented by two Javascript objects :

• the "class dictionary" : a Javascript object with all the attributes and methods defined in the class
• the "factory function" : the function used to create instances

Currently, the Javascript object used for the class A is the "factory function". Setting A.__init__ changes the attribute __init__ of the class dictionary, but not the factory function itself...

Instead of trying workarounds, I have decided to change the whole class implementation. I have done it in the branch "rewrite_classes" of the Github repo. I don't think that calling it a "major rewriting" is an overstatement : 57 files changed, 8,189 additions and 9,084 deletions.

I also made more Python-compliant how user-defined classes are created using metaclasses, how class instances are created using the metaclass __call__ method. I have learned quite a few things on my way...

The version in this branch "rewrite_classes" is not running correctly : it passes all the tests and runs all the demos in the gallery. But it's such a huge change that there are certainly regressions that I didn't see. Before merging into master, I would like as much people as possible to test this new version with their own code, and report issues in the tracker by prepending [rewrite_classes] to the issue name.

If there are no huge problems, I hope to be able to merge at the end of next week.

[Joao S. O. Bueno]

Yes -that is a lot of work -and straight into the "core" of the thing.

Congratulations and thank you.

My major projects using Brython are frozen in time for years now - but
I will see if I can
update then to use this branch.

> --
> You received this message because you are subscribed to the Google Groups
> "brython" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to brython+u...@googlegroups.com.
> To post to this group, send email to bry...@googlegroups.com.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/brython/b2d4897d-49e1-416e-bb96-6c23775baaf9%40googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.

[André]

On Tuesday, 13 February 2018 06:50:40 UTC-4, Pierre Quentel wrote:

In issue #766 Glyph requested the support of the popular attrs package.

When I started working on it I realized that there was a big issue with the way classes are currently implemented.

The following code fails on the version currently on line at brython.info:

class A:

    def __init__(self):
        self.version = "old"

def new_init(obj):
    obj.version = "new"

A.__init__ = new_init

assert A().version == "new"

In the current implementation, classes are represented by two Javascript objects :

• the "class dictionary" : a Javascript object with all the attributes and methods defined in the class
• the "factory function" : the function used to create instances

Currently, the Javascript object used for the class A is the "factory function". Setting A.__init__ changes the attribute __init__ of the class dictionary, but not the factory function itself...

Instead of trying workarounds, I have decided to change the whole class implementation. I have done it in the branch "rewrite_classes" of the Github repo. I don't think that calling it a "major rewriting" is an overstatement : 57 files changed, 8,189 additions and 9,084 deletions.

I also made more Python-compliant how user-defined classes are created using metaclasses, how class instances are created using the metaclass __call__ method. I have learned quite a few things on my way...

The version in this branch "rewrite_classes" is not running correctly : it passes all the tests and runs all the demos in the gallery.

I assume you meant to write "now" instead of "not".   And, if so, I need help to test things out. 

I've tried to get that branch but something has obviously gone wrong as neither the editor nor the console load and I cannot run the turtle demos (which was the first thing I tried)

To get the branch, I did the following:

> git checkout --track upstream/rewrite_classes

Switched to a new branch 'rewrite_classes'

Branch rewrite_classes set up to track remote branch rewrite_classes from upstream.

> git branch

  fix/master/turtle709

  fix/master/turtle739

  master

* rewrite_classes

> git status

On branch rewrite_classes

Your branch is up-to-date with 'upstream/rewrite_classes'.

===

I then started the brython server as usual with "python server.py" and got greeted by a site with a new look (blue background) whose contrast is a bit too low for my taste but which I assume indicates that I am using the right branch.

As an example of what I see, if I attempt to load the Console (from the top menu), here is the first significant (i.e. ignoring the 404 errors) error logged by Chrome:

Error
    at Object._b_.AttributeError (eval at $make_exc (py_exceptions.js:417), <anonymous>:41:15)
    at $JSObjectDict.__getattribute__ (js_objects.js:317)
    at Object.$B.$getattr (py_builtin_functions.js:875)
    at Object.$B.$call (py_utils.js:736)
    at show1 (eval at run_py (py_import.js:253), <anonymous>:203:32)
    at eval (eval at run_script (py2js.js:7944), <anonymous>:16:97)
    at run_script (py2js.js:7944)
    at callback (py2js.js:7886)
    at Object.load_scripts [as _load_scripts] (py2js.js:7922)
    at _run_scripts (py2js.js:8295)
    at brython (py2js.js:8155)
    at onload (console.html?lang=fr:65)

 

Given this massive failure, I assume it is something *I* did wrong, and which should not be filed as an issue on the tracker.

Help?

André

[Pierre Quentel]

Le mardi 13 février 2018 16:20:37 UTC+1, André a écrit :

On Tuesday, 13 February 2018 06:50:40 UTC-4, Pierre Quentel wrote:

In issue #766 Glyph requested the support of the popular attrs package.

When I started working on it I realized that there was a big issue with the way classes are currently implemented.

The following code fails on the version currently on line at brython.info:

class A:

    def __init__(self):
        self.version = "old"

def new_init(obj):
    obj.version = "new"

A.__init__ = new_init

assert A().version == "new"

In the current implementation, classes are represented by two Javascript objects :

• the "class dictionary" : a Javascript object with all the attributes and methods defined in the class
• the "factory function" : the function used to create instances

Currently, the Javascript object used for the class A is the "factory function". Setting A.__init__ changes the attribute __init__ of the class dictionary, but not the factory function itself...

Instead of trying workarounds, I have decided to change the whole class implementation. I have done it in the branch "rewrite_classes" of the Github repo. I don't think that calling it a "major rewriting" is an overstatement : 57 files changed, 8,189 additions and 9,084 deletions.

I also made more Python-compliant how user-defined classes are created using metaclasses, how class instances are created using the metaclass __call__ method. I have learned quite a few things on my way...

The version in this branch "rewrite_classes" is not running correctly : it passes all the tests and runs all the demos in the gallery.

I assume you meant to write "now" instead of "not". 

Oops. When I was young, in the 80's, we would have called that a "laspus révélateur"...

 And, if so, I need help to test things out. 

I've tried to get that branch but something has obviously gone wrong as neither the editor nor the console load and I cannot run the turtle demos (which was the first thing I tried)

To get the branch, I did the following:

> git checkout --track upstream/rewrite_classes
 

Switched to a new branch 'rewrite_classes'

Branch rewrite_classes set up to track remote branch rewrite_classes from upstream.

> git branch

  fix/master/turtle709

  fix/master/turtle739

  master

* rewrite_classes

> git status

On branch rewrite_classes

Your branch is up-to-date with 'upstream/rewrite_classes'.

Since I'm hopeless at git, I just made a fresh clone of the repository, switched to the "rewrite_classes" branch and started the server, all the tests pass and the console loads without errors. The most likely is that *I* am doing something wrong, but I don't know what.

[André]

On Tuesday, 13 February 2018 15:16:57 UTC-4, Pierre Quentel wrote:

Le mardi 13 février 2018 16:20:37 UTC+1, André a écrit :

On Tuesday, 13 February 2018 06:50:40 UTC-4, Pierre Quentel wrote:

...

The version in this branch "rewrite_classes" is not running correctly : it passes all the tests and runs all the demos in the gallery.

I assume you meant to write "now" instead of "not". 

Oops. When I was young, in the 80's, we would have called that a "laspus révélateur"...

"lapsus", sûrement ;-) 

 

 And, if so, I need help to test things out. 

I've tried to get that branch but something has obviously gone wrong as neither the editor nor the console load and I cannot run the turtle demos (which was the first thing I tried)

To get the branch, I did the following:

> git checkout --track upstream/rewrite_classes
 

Switched to a new branch 'rewrite_classes'

Branch rewrite_classes set up to track remote branch rewrite_classes from upstream.

> git branch

  fix/master/turtle709

  fix/master/turtle739

  master

* rewrite_classes

> git status

On branch rewrite_classes

Your branch is up-to-date with 'upstream/rewrite_classes'.

Since I'm hopeless at git, I just made a fresh clone of the repository, switched to the "rewrite_classes" branch and started the server, all the tests pass and the console loads without errors. The most likely is that *I* am doing something wrong, but I don't know what.

I think that *I* am hopeless at git...

I just made a fresh clone of the repository, switched to the "rewrite_classes" branch and started the server.  The console did load properly.. (Yay!)

However ... if I attempt to run the very first turtle demo, I get the following:

Traceback (most recent call last):

  module _collections line 475

    field_names=tuple(map(str,field_names))

  module python_script line 38

    traceback.print_exc()

  module  line 1

    import turtle

  module turtle line 1375

    import inspect

  module inspect line 34

    import imp

  module imp line 30

    import tokenize

  module tokenize line 94

    class TokenInfo(collections. namedtuple('TokenInfo','type string start end line')):

  module _collections line 475

    field_names=tuple(map(str,field_names))

NameError: name 'map' is not defined

So ... progress of sorts, but something is definitely wrong here.

André

[Pierre Quentel]

Le mardi 13 février 2018 20:34:50 UTC+1, André a écrit :

On Tuesday, 13 February 2018 15:16:57 UTC-4, Pierre Quentel wrote:

Le mardi 13 février 2018 16:20:37 UTC+1, André a écrit :

On Tuesday, 13 February 2018 06:50:40 UTC-4, Pierre Quentel wrote:

...

The version in this branch "rewrite_classes" is not running correctly : it passes all the tests and runs all the demos in the gallery.

I assume you meant to write "now" instead of "not". 

Oops. When I was young, in the 80's, we would have called that a "laspus révélateur"...

"lapsus", sûrement ;-) 

Oops again...

I think that *I* am hopeless at git...

I just made a fresh clone of the repository, switched to the "rewrite_classes" branch and started the server.  The console did load properly.. (Yay!)

However ... if I attempt to run the very first turtle demo, I get the following:

Traceback (most recent call last):

  module _collections line 475

    field_names=tuple(map(str,field_names))

  module python_script line 38

    traceback.print_exc()

  module  line 1

    import turtle

  module turtle line 1375

    import inspect

  module inspect line 34

    import imp

  module imp line 30

    import tokenize

  module tokenize line 94

    class TokenInfo(collections. namedtuple('TokenInfo','type string start end line')):

  module _collections line 475

    field_names=tuple(map(str,field_names))

NameError: name 'map' is not defined

So ... progress of sorts, but something is definitely wrong here.

André

My bad... I forgot to update the generated files (brython.js et. al.). Can you try with the latest version ?

[Andre Roberge]

Ok, this works  ... and it appears that what I had reported in issue #782 is fixed as well in this new version.

When I have some more time, I'll have to figure out how to test this with my main application (http:reeborg.ca/reeborg.html) 

André

 

--
You received this message because you are subscribed to a topic in the Google Groups "brython" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/brython/Puefct6H9GQ/unsubscribe.
To unsubscribe from this group and all its topics, send an email to brython+unsubscribe@googlegroups.com.

To post to this group, send email to bry...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/brython/9afb209b-7290-4589-a03f-78474e6ca324%40googlegroups.com.

[Pierre Quentel]

I have merged the branch "rewrite_classes" into master.