Enaml syntaxerror detail

[bburan]

See #529 for details. Note that the unit tests I added do not actually capture the bug for some reason, but I can verify that in my own code (where I discovered the bug), the changes I made to raising SyntaxError solves the issue.

[MatthieuDartiailh]

I am surprised that GHA did not run. Also it looks like your change could break older Python since old syntax error probably won't handle well so many arguments.

[bburan]

I tested down to 3.8 and it did not break. I think SyntaxError was designed to accept *args and then it just ignores the extra ones in earlier versions of Python.

[bburan]

I see the tests are failing. It's possible I did not test correctly. I think we can just add a check for if PY310 to use the new syntax?

[bburan]

FYI, pip install -e .\enaml has not been working for me. I have to do a regular pip install before I can test changes. I'm not sure why.

[MatthieuDartiailh]

What error do you see ?

I have not seen such issues but I usually regenerate the parser using the full path to the script, not sure it should make a difference.

[bburan]

So, when I pip install -e .\enaml in an environment that already had Enaml installed from PyPI, I get the following error:

Traceback (most recent call last):
  File "<frozen runpy>", line 198, in _run_module_as_main
  File "<frozen runpy>", line 88, in _run_code
  File "C:\Users\mmm\miniconda3\envs\psi4\Scripts\cfts.exe\__main__.py", line 4, in <module>
  File "C:\Users\mmm\projects\psi4\src\cfts\cfts\app\main.py", line 3, in <module>
    with enaml.imports():
         ^^^^^^^^^^^^^
AttributeError: module 'enaml' has no attribute 'imports'

However, I just created a fresh environment to test pip install -e .\enaml and it worked fine. So, it is likely a bug with pip. Regardless, this is a separate issue. The main issue is that there's something odd about SyntaxError on Python 3.11 that is preventing it from providing the full details of the error. Without this patch, I just know which file the error is in. With the patch, I know what line the error is on. I pushed a fix to add compatibility for Python 3.8 and 3.9. Not sure why it hasn't shown up yet (it's on the enaml-syntaxerror-detail branch in my repo).

[bburan]

Ok. That last commit finally showed up here and the CI checks seem to have passed successfully for all Python versions. I'm working on another computer and ran into this exact same issue where I keep getting a very non-informative SyntaxError message:

Traceback (most recent call last):
  File "C:\Users\mmm\miniconda3\envs\psi1\lib\runpy.py", line 196, in _run_module_as_main
    return _run_code(code, main_globals, None,
  File "C:\Users\mmm\miniconda3\envs\psi1\lib\runpy.py", line 86, in _run_code
    exec(code, run_globals)
  File "C:\Users\mmm\miniconda3\envs\psi1\Scripts\noise-exp.exe\__main__.py", line 4, in <module>
  File "C:\Users\mmm\projects\psi1\src\noise-exp\noise_exp\main.py", line 4, in <module>
    from .gui import Main
  File "<frozen importlib._bootstrap>", line 1027, in _find_and_load
  File "<frozen importlib._bootstrap>", line 1006, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 688, in _load_unlocked
  File "C:\Users\mmm\miniconda3\envs\psi1\lib\site-packages\enaml\core\import_hooks.py", line 139, in exec_module
    code, _ = self.get_code()
  File "C:\Users\mmm\miniconda3\envs\psi1\lib\site-packages\enaml\core\import_hooks.py", line 400, in get_code
    return self.compile_code()
  File "C:\Users\mmm\miniconda3\envs\psi1\lib\site-packages\enaml\core\import_hooks.py", line 364, in compile_code
    ast = parse(self.read_source(), file_info.src_path)
  File "C:\Users\mmm\miniconda3\envs\psi1\lib\site-packages\enaml\core\parser\__init__.py", line 66, in parse
    return _parse(
  File "C:\Users\mmm\miniconda3\envs\psi1\lib\site-packages\enaml\core\parser\__init__.py", line 42, in _parse
    return parser.parse("start")
  File "C:\Users\mmm\miniconda3\envs\psi1\lib\site-packages\enaml\core\parser\base_python_parser.py", line 79, in parse
    raise SyntaxError("invalid syntax", (self.filename, token.start, 0, token.line))
SyntaxError: invalid syntax (gui.enaml)

Incorporating this patch fixes it, giving me:

Traceback (most recent call last):
  File "C:\Users\mmm\miniconda3\envs\psi1\lib\runpy.py", line 196, in _run_module_as_main
    return _run_code(code, main_globals, None,
  File "C:\Users\mmm\miniconda3\envs\psi1\lib\runpy.py", line 86, in _run_code
    exec(code, run_globals)
  File "C:\Users\mmm\miniconda3\envs\psi1\Scripts\noise-exp.exe\__main__.py", line 4, in <module>
  File "C:\Users\mmm\projects\psi1\src\noise-exp\noise_exp\main.py", line 4, in <module>
    from .gui import Main
  File "<frozen importlib._bootstrap>", line 1027, in _find_and_load
  File "<frozen importlib._bootstrap>", line 1006, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 688, in _load_unlocked
  File "C:\Users\mmm\miniconda3\envs\psi1\lib\site-packages\enaml\core\import_hooks.py", line 139, in exec_module
    code, _ = self.get_code()
  File "C:\Users\mmm\miniconda3\envs\psi1\lib\site-packages\enaml\core\import_hooks.py", line 400, in get_code
    return self.compile_code()
  File "C:\Users\mmm\miniconda3\envs\psi1\lib\site-packages\enaml\core\import_hooks.py", line 364, in compile_code
    ast = parse(self.read_source(), file_info.src_path)
  File "C:\Users\mmm\miniconda3\envs\psi1\lib\site-packages\enaml\core\parser\__init__.py", line 66, in parse
    return _parse(
  File "C:\Users\mmm\miniconda3\envs\psi1\lib\site-packages\enaml\core\parser\__init__.py", line 42, in _parse
    return parser.parse("start")
  File "C:\Users\mmm\miniconda3\envs\psi1\lib\site-packages\enaml\core\parser\base_python_parser.py", line 79, in parse
    raise SyntaxError("invalid syntax", (self.filename, *token.start, token.line, *token.end))
  File "C:\Users\mmm\projects\psi1\src\noise-exp\noise_exp\gui.enaml", line 18
    )
SyntaxError: invalid syntax

This is on Python 3.10.9 (the system I originally discovered this error on is Python 3.11).

[MatthieuDartiailh]

Could you add an entry to the changelog and commit a regenerated parser ? I think the fact we changed the base but did not update the parser is why appveyor is complaining but I find it a bit weird.

[frmdstryr]

I added detail for indentation errors. Could you try it & cherry pick from https://github.com/frmdstryr/enaml/tree/indexerror-detail ?

For some reason the generated parser is formatted weirdly so I didn't commit it.

[MatthieuDartiailh]

You need to manually run black after generating it.

[MatthieuDartiailh]

The CIs look unhappy. I should be able to look at this during the week.

[bburan]

@MatthieuDartiailh Sorry for the long delay. Finally got around to fixing the unit tests. One is still failing (upload to codecov), but I don't think that's relevant to the changes made in this commit.

[MatthieuDartiailh]

Thanks ! Could you add a changelog entry and a regenerated parser (formatted with black) ?

[bburan]

@frmdstryr The errors above are related to the IndexError tests you added. It seems that Enaml is properly detecting that there is a SyntaxError; however, it's being reported as a general SyntaxError rather than an IndentationError. I'm not sure how to fix this. I also switched to your branch and tested on Python 3.10 and it's not working either. Only two of the tests (childdef-indent-mismatch and class) are being reported as IndentationErrors. All others are SyntaxErrors. Thoughts?

[frmdstryr]

Odd... seems like something with pegen 0.2.0. If you revert to pip install pegen==0.1.0 and regen the parser they all pass.

I guess the parser with the latest pegen is not calling raise_indentation_error.

[bburan]

@frmdstryr That explains why the tests passed at one point, but then did not pass. I was doing a line-by-line comparison and was stumped. Thanks for catching that. @MatthieuDartiailh, any thoughts? I'm not familiar with pegen, so I'm not sure where to start and whether this is a bug in Enaml or in Pegen.

[MatthieuDartiailh]

Switching to pegen 0.2.0 is somewhere on my list of things to do for 3.12. Right now I have bytecode running and atom is close but CI segfault when exiting. Next I will look into pegen (f-string require some extra work) and finally reach enaml but it will take time. For this PR simply regen with 0.1.0 and commit. You can also pin pegen if I forgot to do it.

[codecov[bot]]

Codecov Report

Merging #530 (c91b3e7) into main (87e9c31) will increase coverage by 0.08%. Report is 1 commits behind head on main. The diff coverage is 100.00%.

:exclamation: Current head c91b3e7 differs from pull request most recent head 236da37. Consider uploading reports for the commit 236da37 to get more accurate results

Additional details and impacted files

```diff @@ Coverage Diff @@ ## main #530 +/- ## ========================================== + Coverage 72.65% 72.73% +0.08% ========================================== Files 287 287 Lines 24417 24425 +8 Branches 4343 4344 +1 ========================================== + Hits 17740 17766 +26 + Misses 5591 5580 -11 + Partials 1086 1079 -7 ```

:loudspeaker: Have feedback on the report? Share it here.

[bburan]

Looks like it passed, finally. :tada: Thanks @MatthieuDartiailh and @frmdstryr for your help!

[MatthieuDartiailh]

Thanks @bburan and @frmdstryr