cirosantilli / vcdvcd

Python Verilog value change dump (VCD) parser library + the nifty vcdcat VCD command line pretty printer.
Other
54 stars 21 forks source link

Clarify Supported Python Versions #20

Closed GCHQDeveloper560 closed 3 years ago

GCHQDeveloper560 commented 3 years ago

On an older (Ubuntu 18.04) system where the default python is still 2.7, vcdcat fails with the error

Traceback (most recent call last):
  File "./vcdcat", line 9, in <module>
    import vcdvcd
  File "/<snip>/vcdvcd/vcdvcd/__init__.py", line 1, in <module>
    from .vcdvcd import *
  File "/<snip>/vcdvcd/vcdvcd/vcdvcd.py", line 384
    def __contains__(self, o: object) -> bool:
                            ^
SyntaxError: invalid syntax

There's certainly no reason not to move to Python 3.6+, but would it make sense to change the opening line of vcdcat and similar to #!/usr/bin/env python3? That change at least works on the system in question.

GCHQDeveloper560 commented 3 years ago

I made the above change and realised that the tests fail using Python 3.6 with errors like

======================================================================
ERROR: test_toplevel_signal (__main__.Test)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "./test.py", line 132, in test_toplevel_signal
    signal = vcd['clock']
  File "/mnt/jksmith-home/fpga_simulation/vcdvcd/vcdvcd/vcdvcd.py", line 252, in __getitem__
    if isinstance(refname,re.Pattern):
AttributeError: module 're' has no attribute 'Pattern'

It appears that re.Pattern is only supported in Python 3.7+. I'm not sure of the correct way to do the check in question on older releases. The type of patterns is a class clearly not meant for public consumption:

Python 3.6.9 (default, Oct  8 2020, 12:12:24)
[GCC 8.4.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import re
>>> type(re.compile('out.*'))
<class '_sre.SRE_Pattern'>

Do you intend to support only Python 3.7+, or is a change needed to support 3.6? I'll edit the issue title since the issue is wider than the shebang lines above.