enaml icon indicating copy to clipboard operation
enaml copied to clipboard

[BUG] Upgrading from 0.15.2 to 0.16.x goes boom, boom, boom

Open Kochise opened this issue 2 years ago • 9 comments

Enaml app, using @enamlx as well, worked nice on Feb 20th 2023, but then 0.16.0 was published :

Install

pip install --no-cache-dir --upgrade pip

pip install --no-cache-dir --upgrade qtpy
pip install --no-cache-dir --upgrade qasync
pip install --no-cache-dir --upgrade enaml

pip install --no-cache-dir --upgrade pyqtgraph
pip install --no-cache-dir --upgrade enamlx

pip install --no-cache-dir --upgrade asyncua
pip install --no-cache-dir --upgrade opcua
pip install --no-cache-dir --upgrade opcua-webclient
pip install --no-cache-dir --upgrade opcua-client
pip install --no-cache-dir --upgrade opcua-modeler
pip install --no-cache-dir --upgrade "git+https://github.com/PrediktorAS/opcua-tools.git"

export PATH=$PATH:/home/Kochise/.local/bin 
export QT_QPA_PLATFORM=wayland

Run

ph2_Baseline_cm7_client_view.enaml.txt

enaml-run ph2_Baseline_cm7_client_view.enaml

Cry

There are some external files (icons for the tree) that are missing but not mandatory to get the error.

ph2_Baseline_cm7_client_view.error.txt

Regards.

Kochise avatar May 23 '23 13:05 Kochise

Look like you uncovered a bug in the new parser based on pegen. I will try to investigate ASAP.

MatthieuDartiailh avatar May 23 '23 13:05 MatthieuDartiailh

A quick investigation points to fn_opcua_get_tree as the function that we fail to parse

MatthieuDartiailh avatar May 23 '23 13:05 MatthieuDartiailh

fn_opcua_get_tree

Such a beautiful function though 😢

Kochise avatar May 23 '23 13:05 Kochise

I have one likely suspect but I will dive deeper later today. I will keep you posted.

MatthieuDartiailh avatar May 23 '23 13:05 MatthieuDartiailh

Okay, so my first suspicion was silly, and you simply suffer from the deeply recursive nature of the parser. One workaround woud be to avoid so many nested comprehensions.

A solution would consist in adopting the trampoline pattern as used in bytecode when computing stack depth. The bad news is that it requires some fundamental changes into how pegen generates parsers. I will look into it though.

MatthieuDartiailh avatar May 23 '23 16:05 MatthieuDartiailh

Thank. I don't see how to improve the enamlx tree's walker with generic depth traversal algorithm.

It worked in enaml 0.15.2 though.

What were the reasons to switch to the pegen parser again ?

Kochise avatar May 24 '23 08:05 Kochise

Hi, any news on this ?

Kochise avatar Oct 25 '23 12:10 Kochise

I have been busy working on Python 3.12 support so there has been no definitive fix for this.

However I believe you could easily refactor your code to use a factory function to generate the list of children and replace the deep syntactic nesting by some recursive calls. Something like:

def create_children(infos):
	return [EnamlxNode(
		str_text = f"{inner['name']}",
		list_data = [inner['id'], inner['cls'], inner['type']],
		list_node = create_children[inner['children']]
	) for inner in infos]*

MatthieuDartiailh avatar Oct 30 '23 16:10 MatthieuDartiailh

Thank for the feedback, I'll give it a try.

Kochise avatar Nov 09 '23 09:11 Kochise