gvanrossum / gvanrossum.github.io

BDFL website
108 stars 21 forks source link

scopes.py fails its own test(). #19

Open mrolle45 opened 2 years ago

mrolle45 commented 2 years ago

I ran this module standalone, and in the test() method there was an AssertionError. Several of the calls to Scope.lookup() return different values from what was expected. Here's a list of them. These lines replace the similar lines. The module now runs without exceptions:

    assert foo.lookup("C") is None # not global
    assert c.lookup("self") is global # not None
    assert c.lookup("a") is global # not None
    assert c.lookup("blah") is global # not None
    assert c.lookup("x") is global # not None

I don't know whether the lookup() calls are wrong, or the assert statements are wrong.

gvanrossum commented 2 years ago

Hm... I guess this is because I stopped using the script as its own test. My test process (last I looked at this code) was to run build.py, which takes a file argument (defaulting to default.py), parses it, and prints the scopes collected from it. I guess I changed parts of the API since I last ran scopes.py itself. Sorry about that!

gvanrossum commented 2 years ago

Yeah, it looks like any lookup from the class scope of a name that's not defined in that class scope now immediately returns globals rather than None, whereas looking up a global from a function scope, even if it's defined, returns Noneinstead ofglobals`. :-)

I think the former is explained by what may be a bug in the last committo scopes.py. If I add this patch the c.lookup() calls all return None again:

--- a/formal/scopes.py
+++ b/formal/scopes.py
@@ -90,7 +90,7 @@ class OpenScope(Scope):
             if s is not None:
                 return s.lookup(name)
             else:
-                return self.global_scope()
+                return self.global_scope().lookup(name)

 class GlobalScope(OpenScope):

But, believe it or not, I have not yet fully understood my own code again, so I'll refrain from making this change.

Next I need to understand why foo.lookup("C") has started returning None. This can be explained by an earlier commit to scopes.py. Here the code may be right and the test wrong.

But again, I am not sure yet. Why should looking up a name that won't be found sometimes return None and sometimes the global scope? I have a feeling that it has to do with how things interact with e.g. nonlocal and global statements. But it may also be that I was merely being inconsistent.

Could you help me by trying to read my code and seeing for yourself what you expect the answer to be in some of these cases?

mrolle45 commented 2 years ago

I'm going to test your code thoroughly by incorporating it into my mega test script, the one that generates and then executes a python module with 26,000 test cases. I'll be comparing the value predicted by the scope object with the actual value at runtime. First, I'm going to make a PR, now that I'm learning how to use Git and use it in Visual Studio, what will put in the correct values for the assert statements. This will verify that I've learned how to do it correctly. I'll nominate you as reviewer. I'd appreciate your letting me know the PR was done correctly, or what is incorrect about it. You might want to hold off applying it until I see what bugs there are in the scope objects. Not much point in comparing a lookup() result with the wrong value.

gvanrossum commented 2 years ago

Thanks, that sounds like a plan.

mrolle45 commented 2 years ago

Help! I'm in Visual Studio. I've created a branch in my clone repo, made the change to scopes.py, staged the file and made a commit on the branch. I don't know if you use VS or not. Anyway, VS can show PRs in my repo, and has a Create New action. This dialog has a dropdown list to select a branch, but it only shows main. It should show my local branch too but it doesn't. I enter a name and description for the new PR. However, the 'Create pull request' button is disabled.

I could try making my PR using the command line, like git push .... However, I found the help page forgit push really complicated and bewildering. What do you actually do when you want to add a PR to the GitHub repo?

gvanrossum commented 2 years ago

I use the git command line. Maybe you can study git without my help? It's really not my strength teaching people how to use git.