| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
Co-authored-by: AN Long <aisk@users.noreply.github.com>
|
I'll fix this in a while, my apologies for quite a big fault in the code actually |
Sorry, something went wrong.
| outer = find_block(top, "outer") | ||
| self.assertIn("x",outer.get_cells()) | ||
| self.assertTrue(outer.lookup("x").is_cell()) | ||
| self.assertFalse(outer.lookup("inner").is_cell()) |
There was a problem hiding this comment.
Let's put this test next to test_free.
Sorry, something went wrong.
|
|
||
| def get_cells(self): | ||
| """Return a list of cell variable names in the table.""" | ||
| return [s.get_name() for s in self.get_symbols() if s.is_cell()] |
There was a problem hiding this comment.
This probably belongs next to get_frees.
Sorry, something went wrong.
|
@iritkatriel I was forced to force push the symtable branch due to me updating the branch before for the CI. |
Sorry, something went wrong.
|
|
||
| def get_cells(self): | ||
| """Return a list of cell variable names in the table.""" | ||
| return [s.get_name() for s in self.get_symbols() if s.is_cell()] |
There was a problem hiding this comment.
Why does this not follow the pattern of get_frees and get_nonlocals?
Sorry, something went wrong.
There was a problem hiding this comment.
We'll have to use the _cells variable then, I'll add it.
Sorry, something went wrong.
| code="""def outer(): | ||
| x=1 | ||
| def inner(): | ||
| return x""" |
There was a problem hiding this comment.
the other tests don't do all this. Can we follow the same pattern in this one?
Sorry, something went wrong.
There was a problem hiding this comment.
@iritkatriel I've implemented the change in symtable.py with get_cells
But the current test_cells seems to be the only one that keeps working after I've change it up quite a few times.
Is it really a requirement for it all to have the same pattern?
Sorry, something went wrong.
There was a problem hiding this comment.
Unless there's a reason why we can't, then we should follow the same pattern. What's the difference between the cases?
Sorry, something went wrong.
There was a problem hiding this comment.
self.internal has x as FREE var (for test_free). we need x as CELL var (outer scope). setUp doesn't have that so inline code is needed. However I've removed the unnecessary comments and changed the rest of it to
top=symtable.symtable(code,"?","exec")
outer = find_block(top, "outer")
self.assertEqual(outer.get_cells(), ["x"])
self.assertTrue(outer.lookup("x").is_cell())
self.assertFalse(outer.lookup("inner").is_cell()) to use assert.Equal,true and false like the rest of the functions.
WOuld this be satisfactory as a commit?
Sorry, something went wrong.
There was a problem hiding this comment.
self.spam.lookup("x").is_cell()
Sorry, something went wrong.
There was a problem hiding this comment.
and get_cells test should be next to get_frees test.
Sorry, something went wrong.
There was a problem hiding this comment.
and get_cells test should be next to get_frees test.
isn't test_cells already next to test_free?
Sorry, something went wrong.
| self.assertFalse(outer.lookup("inner").is_cell()) | ||
| self.assertTrue(self.spam.lookup("x").is_cell()) | ||
|
|
||
|
|
There was a problem hiding this comment.
get_cells is not tested at all.
Sorry, something went wrong.
There was a problem hiding this comment.
Add it to test_function_info:
@@ -255,6 +255,7 @@ def test_function_info(self):
self.assertEqual(sorted(func.get_locals()), expected)
self.assertEqual(sorted(func.get_globals()), ["bar", "glob", "some_assigned_global_var"])
self.assertEqual(self.internal.get_frees(), ("x",))
+ self.assertEqual(self.spam.get_cells(), ("some_var", "x",))
Sorry, something went wrong.
|
Can you also update symtable.rst? and add a What's New entry? (I did so for what I changed in 3.14) And once you update the docs, update the NEWS entry to point them to |
Sorry, something went wrong.
Co-authored-by: Irit Katriel <1055913+iritkatriel@users.noreply.github.com>
Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
|
|
||
| .. method:: get_cells() | ||
|
|
||
| Return a tuple containing the names of cell variables in this table. |
There was a problem hiding this comment.
try formatting like the entry for get_frees with :term:
Sorry, something went wrong.
| @@ -0,0 +1 @@ | |||
| Add symtable.is_cell() and get_cells() methods for cell variable analysis. | |||
There was a problem hiding this comment.
There are two News files now
Sorry, something went wrong.
Co-authored-by: Irit Katriel <1055913+iritkatriel@users.noreply.github.com>
Co-authored-by: Irit Katriel <1055913+iritkatriel@users.noreply.github.com>
| Return a tuple containing names of :term:`free (closure) variables <closure variable>` | ||
| in this function. | ||
|
|
||
| .. method:: get_cells() |
There was a problem hiding this comment.
Please add the versionadded directive as well
Sorry, something went wrong.
| @@ -0,0 +1,2 @@ | |||
| Add :meth:`symtable.Function.get_cells` and :meth:`symtable.Symbol.is_cell` methods. | |||
|
|
|||
There was a problem hiding this comment.
Remove the extra empty line
Sorry, something went wrong.
Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
Co-authored-by: Irit Katriel <1055913+iritkatriel@users.noreply.github.com>
Co-authored-by: Irit Katriel <1055913+iritkatriel@users.noreply.github.com>
|
@iritkatriel , @picnixz Thanks for the help |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
Expose cell status in symtable.Symbol via new is_cell() predicate and add get_cells() method to SymbolTable.
Changes
Tests