Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug] Error when importing pyGHDL packages #77

Open
pidgeon777 opened this issue Mar 27, 2024 · 8 comments
Open

[Bug] Error when importing pyGHDL packages #77

pidgeon777 opened this issue Mar 27, 2024 · 8 comments
Assignees

Comments

@pidgeon777
Copy link

I tried to run this code:

from pathlib import Path
from pyGHDL.dom.NonStandard import Design, Document

but the second import crashes:

(common) C:\Work\Scripts\Python\PyGHDL>python test.py        
Traceback (most recent call last):
  File "C:\Work\MEGA\Scripts\Python\PyGHDL\test.py", line 2, in <module>
    from pyGHDL.dom.NonStandard import Design, Document
  File "C:\Users\<USERNAME>\scoop\apps\miniconda3\current\envs\common\Lib\site-packages\pyGHDL\dom\NonStandard.py", line 43, in <module>
    from pyGHDL.dom.Names import SimpleName
  File "C:\Users\<USERNAME>\scoop\apps\miniconda3\current\envs\common\Lib\site-packages\pyGHDL\dom\Names.py", line 37, in <module>
    from pyVHDLModel.Name import Name
  File "C:\Users\<USERNAME>\scoop\apps\miniconda3\current\envs\common\Lib\site-packages\pyVHDLModel\__init__.py", line 60, in <module>
    from pyTooling.Graph           import Graph, Vertex, Edge
  File "C:\Users\<USERNAME>\scoop\apps\miniconda3\current\envs\common\Lib\site-packages\pyTooling\Graph\__init__.py", line 63, in <module>
    from pyTooling.Tree        import Node
  File "C:\Users\<USERNAME>\scoop\apps\miniconda3\current\envs\common\Lib\site-packages\pyTooling\Tree\__init__.py", line 100, in <module>
    class Node(Generic[IDType, ValueType, DictKeyType, DictValueType], metaclass=ExtendedType, useSlots=True):
  File "C:\Users\<USERNAME>\scoop\apps\miniconda3\current\envs\common\Lib\site-packages\pyTooling\MetaClasses\__init__.py", line 250, in __new__
    members['__slots__'] = self.__getSlots(baseClasses, members)
                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "C:\Users\<USERNAME>\scoop\apps\miniconda3\current\envs\common\Lib\site-packages\pyTooling\MetaClasses\__init__.py", line 431, in __getSlots
    raise AttributeError(f"Base-class '{base.__name__}' has no '__slots__'.")
AttributeError: Base-class 'Generic' has no '__slots__'.

I will provide any additional information if needed.

@pidgeon777
Copy link
Author

pip list output:

Package                 Version
----------------------- --------
archspec                0.2.1
argcomplete             3.2.3
boltons                 23.0.0
certifi                 2024.2.2
cffi                    1.16.0
charset-normalizer      2.0.4
colorama                0.4.6
conda                   24.1.0
conda-content-trust     0.1.3
conda-libmamba-solver   24.1.0
conda-package-handling  2.2.0
conda_package_streaming 0.9.0
cryptography            39.0.1
distro                  1.8.0
gitdb                   4.0.11
GitPython               3.1.42
idna                    3.4
jsonpatch               1.32
jsonpointer             2.1
libmambapy              1.5.6
menuinst                2.0.2
packaging               23.1
pip                     23.1.2
platformdirs            3.10.0
pluggy                  1.0.0
pyAttributes            2.5.1
pycosat                 0.6.6
pycparser               2.21
pyGHDL                  4.0.0
pyTooling               4.0.1
pyVHDLModel             0.25.1
requests                2.31.0
ruamel.yaml             0.17.21
ruamel.yaml.clib        0.2.6
setuptools              67.8.0
six                     1.16.0
smmap                   5.0.1
tqdm                    4.65.0
truststore              0.8.0
urllib3                 2.1.0
wheel                   0.38.4
zstandard               0.19.0

@BenWiederhake
Copy link

BenWiederhake commented Oct 9, 2024

The issue is somehow in pyTooling 4.0.1, and I have no hope that any fix will be backported.

This issue can be monkey-patched by changing in .../site-packages/pyTooling/Graph/__init__.py:204:

 @export
 class Base(
 	Generic[DictKeyType, DictValueType],
-	metaclass=ExtendedType, useSlots=True
+	metaclass=ExtendedType, useSlots=False
 ):

and in .../site-packages/pyTooling/Tree/__init__.py:100:

 @export
-class Node(Generic[IDType, ValueType, DictKeyType, DictValueType], metaclass=ExtendedType, useSlots=True):
+class Node(Generic[IDType, ValueType, DictKeyType, DictValueType], metaclass=ExtendedType, useSlots=False):
 	"""
 	A **tree** data structure can be constructed of ``Node`` instances.

This presumably breaks some features, but pyGHDL seems to run okay at first glance. Again, this is a monkey-patch, and the correct fix is to upgrade to a working version of pyTooling.

EDIT: pyTooling version 6.7.0 is definitely okay. You can check other versions for yourself by running python3 -c 'from pyTooling.Tree import Node').

@Paebbels
Copy link
Member

Hi. I'm currently working on a reworked GHDL pipeline, which will also include some dependency updates. You can find this work here: https://github.com/Paebbels/ghdl/tree/simplify-pipeline

It uses pyTooling 6.6.


Disabling slots, doesn't change functionality, just speed. ExtendedTypes has the capability to automatically infer slots from code and thus speed up object creation, execution and reduce memory footprint. It also allows for safer code.

The problem mentioned by @pidgeon777 is related to an incompatible change in Python itself. pyTooling needed to add some special handling for typing.Generic es it doesn't follow some standard Python rules when it comes to slots.

@pidgeon777
Copy link
Author

Hi. I'm currently working on a reworked GHDL pipeline, which will also include some dependency updates. You can find this work here: https://github.com/Paebbels/ghdl/tree/simplify-pipeline

It uses pyTooling 6.6.

Disabling slots, doesn't change functionality, just speed. ExtendedTypes has the capability to automatically infer slots from code and thus speed up object creation, execution and reduce memory footprint. It also allows for safer code.

The problem mentioned by @pidgeon777 is related to an incompatible change in Python itself. pyTooling needed to add some special handling for typing.Generic es it doesn't follow some standard Python rules when it comes to slots.

Great! Thanks for the hard work!

Do you think there is a chance it will be merged on the main ghdl branch?

Also, if I understand correctly, your push will solve the pyTooling import crash discussed in this issue, making pyGHDL usable?

@Paebbels
Copy link
Member

Do you think there is a chance it will be merged on the main ghdl branch?

The merge will be soon. There is also a transition strategy from old to the new pipeline: (1) both in parallel, (2) remove redundant parts, (3) remove unused scripts needed by the old pipeline, ...

Also, if I understand correctly, your push will solve the pyTooling import crash discussed in this issue, making pyGHDL usable?

Yes and no ... depends on what you use/need.
pyTooling will be upgraded to v6.6 or a newer version, so this problem is gone. I cannot guarantee on Python 3.13 compatibility, because testing of that started in parallel for pyTooling and other packages.

Then there is pyGHDL and especially pyGHDL.DOM itself. All features existing to this point should work - as tested by CI/CD and testcases. While pyGHDL fall behind, GHDL and it's testcases advanced. Especially the all08.vhdl as a testcases has new syntax, which causes problems with pyGHDL.DOM and with the CI pipeline to complete.

I might disable one or two checks to get it released, and start from there with further adaptions. It's hard to translate the IIR, when there is barely a documentation :).

@Paebbels Paebbels self-assigned this Oct 22, 2024
@pidgeon777
Copy link
Author

Hi Paebbels,

Thank you for the prompt reply. Given the current state of the transition and the upcoming merge, would you recommend waiting for the merge into the main GHDL repository before attempting to import pyGHDL again to see if the error discussed in this issue still persists?

Just to tackle one problem at a time.

@Paebbels
Copy link
Member

If you want to try, have a look here (latest pipeline): https://github.com/Paebbels/ghdl/actions/runs/11318447350

Here you see: GHDL got build as libghdl and installed as a platform specific wheel to the target system. Then it checks the proper installation of e.g. the LSP executable or the pyGHDL.DOM test utility. Afterwards the LSP and libghdl testsuites are executed (DOM is disabled).

If I'm right, there error reported in the issue would have been triggered already by these testcases, independent of the disabled DOM tests. You can activate DOM tests in your local setup and it will report the translation issue.

image

@pidgeon777
Copy link
Author

Thank you, I'm going to give it a look once I finish my tasks 👍.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants