beeware / batavia

A JavaScript implementation of the Python virtual machine.
http://pybee.org/batavia
Other
1.39k stars 424 forks source link

[Removed venv changed] Fix pystone #696

Closed Boeing737ng closed 6 years ago

Boeing737ng commented 6 years ago

------------------------------- Describe your changes in detail -----------------------------------

Previously, I wrote an issue #687 (currently closed) which was duplicate of one of a comment in #602. It's about odd functionality of PyStone.

When we press the pystone button, it appears to bring pystone.py file from our own PC path. The parameters from the button ('5', '500', '50000') wasn't passing correctly to that file so the pystone only gave the benchmark result of 50000 loops which is a default value in pystone.py.

To fix it, I created a copy file of pystone.py and located in /testserver directory(same as sample.py directory) and made some change to get the parameter value which is defined as "sys.argv". I also edited the parameter bytecode(pystone.__file__) to bytecode(pystone.py) in urls.py so now it calls the file directly from it's current directory.

----------------------------- What problem does this change solve? -----------------------------

As I mentioned in a closed issue #687, Pystone buttons only appeared to give a result of "50000" loops no matter which pystone button we click. Now it gives a correct result depends on its given number of loops(passes by parameter).

------------------------------------------- Fixed issue ------------------------------------------- It Fixes #687

Please suggest if I need to do something else for this changes

I have removed venv changes and only submitting the specific fixes

Boeing737ng commented 6 years ago

all right, i’ll go through again and try to solve it with correct approach

timb07 commented 6 years ago

The real question is why this change is needed at all - why isn't the "elif" statement on line 269 catching this case? What else is being passed in as an argument, and why?

This is sys.argv: ['batavia', '5']

Why? I don't know.

timb07 commented 6 years ago

Why? I don't know.

Here it is, in VirtualMachine.js:531:

        // Set up sys.argv
        sys.argv = new types.List(['batavia'])
        if (args) {
            sys.argv.extend(args)
        }
timb07 commented 6 years ago

Okay, ignore the above comments—they're not relevant. But they led to finding the actual problem:

    nargs = len(sys.argv) - 1
    if nargs > 1:
        error("%d arguments are too many;" % nargs)
    elif nargs == 1:
    [...]

nargs == 1 evaluates to False, even though when printing the value of nargs, it looks like 1.

The problem is here in types/List.js:59:

List.prototype.__len__ = function() {
    return this.length
}

this.length is a JS number, not a Batavia Int.