Add a proper dynamicscope iterator

[frmdstryr]

It is not very efficient to the use of dir() and a set to track used keys... but makes python 3.12+ provide useful messages for name errors and attribute errors.

I'd like to add more tests after any initial feedback.

Eg:

enamldef Main(Window): window:
    Container:
        PushButton:
            text = "Click me!"
            clicked :: print(windo.size())

Gives NameError: name 'windo' is not defined. Did you mean: 'window'?

From looking at the traceback source code it ignores giving suggestions if there are > 750 items (https://github.com/python/cpython/blob/main/Lib/traceback.py#L1465). Which I can see it easily exceeding.. So it might be good to also skip globals and builtins?

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 70.96%. Comparing base (e00ca76) to head (07fedc2).
Report is 2 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #571      +/-   ##
==========================================
+ Coverage   70.92%   70.96%   +0.03%     
==========================================
  Files         287      288       +1     
  Lines       25698    25733      +35     
  Branches     3641     3645       +4     
==========================================
+ Hits        18226    18261      +35     
  Misses       6384     6384              
  Partials     1088     1088              

[MatthieuDartiailh]

Do we agree this supersedes #566 ?

[frmdstryr]

Yes, should I close that or push this on top of that branch?

[MatthieuDartiailh]

You can close it, I just wanted to be sure.

[sccolbert]

I think most of this should be done in a Python subclass of DynamicScope? It doesn't need to be fast, just correct. It's much easier to do that from Python. For example:

from itertools import chain, repeat
from _dynamicscope import _DynamicScope


"""
_DynamicScope is a C++ class which exposes the following attributes:

_f_writes
_self_str
_change_str
_f_locals
_f_globals
_f_builtins

"""

class DynamicScope(_DynamicScope):

	def __iter__(self):
		fwrites_it = iter(self._f_writes)
		self_it = repeat(self._self_str, 1)
		change_it = repeat(self._change_str, 1)
		flocals_it = iter(self._f_locals)
		fglobals_it = iter(self._f_globals)
		fbuiltins_it = iter(self._f_builtins)
		parent_it = iter(self._parent)

		return chain(
			fwrites_it,
			self_it,
			change_it,
			flocals_it,
			fglobals_it,
			fbuiltins_it,
			parent_it
		)

	def keys(self):
		return iter(self)

	def values(self):
		return (self[key] for key in self)

	def items(self):
		return ((key, self[key]) for key in self)

	def update(self, items):
		for (key, value) in items:
			self[key] = value

[frmdstryr]

Is it ok to expose the internal fields as properties? I guess it'd be fine with only getters.

[sccolbert]

Yes. Make them read-only properties with a leading underscore. i.e. they are "just an implementation detail" that the public shouldn't mess with.

…

On Thu, Jan 30, 2025 at 2:14 PM frmdstryr ***@***.***> wrote: Is it ok to expose the internal fields as properties? I guess it'd be fine with only getters. — Reply to this email directly, view it on GitHub <#571 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AABBQSNQJBZQWWU4X3R3RZL2NKB3BAVCNFSM6AAAAABWDFZNF2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDMMRVGQ4DGNJXGA> . You are receiving this because you commented.Message ID: ***@***.***>

[sccolbert]

Why traverse the parent tree here ahead-of-time instead of allowing chain to naturally recurse into the parent iterator as needed?

[frmdstryr]

Not sure I follow... the owner is a Declarative, the parent is another Declarative neither are iterable. It could use traverse_ancestors?

Since the traceback module uses list(frame.f_locals), if it doesn't remove duplicates it will likely quickly overflow the 750 item limit and not provide a suggestion at all...

[frmdstryr]

Or is there a way to get the parent dynamic scope? I don't see it anywhere from a quick look.

[sccolbert]

Sorry, yeah, I had it in my head that parent was another DynamicScope object. That said, I think we still put all of that logic into a generator function, instead of computing ahead-of-time.

[sccolbert]

But yeah, traverse_ancestors could part of a generator function that aggregates the keys while only yielding newly-seen keys.

[sccolbert]

For the 750 item limit, that really should be an issued raised with the core traceback module. Instead of 100% punting at 751 names, they should truncate their search space to the first 750 items, and just ignore the rest.

[sccolbert]

Do we need an update method? All the rest of the methods are readonly. I know the C++ class implements __setitem__, but IIRC that's in order to handle local assignments. I'm not sure we really need this method.

[frmdstryr]

Technically no... I added it to support ipython embedding since it attempts to update the scope when exiting an embedded console.

[sccolbert]

trailing comma?

[sccolbert]

Should filter all keys with a single leading _ instead of just double? i.e. only yield the "public" keys.