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

Representer fails on some solutions #48

Closed
ErikSchierboom opened this issue Mar 1, 2024 · 7 comments
Closed

Representer fails on some solutions #48

ErikSchierboom opened this issue Mar 1, 2024 · 7 comments

Comments

@ErikSchierboom
Copy link
Member

Whilst looking into https://forum.exercism.org/t/solution-isnt-displayed-in-community-solutions/10071, I found that the representer crashes on this solution: https://exercism.org/tracks/python/exercises/sgf-parsing/solutions/Timus

Traceback (most recent call last):
  File "/opt/representer/bin/run.py", line 56, in <module>
    main()
  File "/opt/representer/bin/run.py", line 52, in main
    representer.represent(args.slug, args.input, args.output)
  File "/opt/representer/representer/__init__.py", line 99, in represent
    txt_dst.write_text(representation.dump_code())
                       ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/representer/representer/__init__.py", line 37, in dump_code
    code = utils.to_source(self._tree)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/representer/representer/utils.py", line 98, in to_source
    return astor.to_source(tree)
           ^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/astor/code_gen.py", line 62, in to_source
    generator.visit(node)
  File "/usr/local/lib/python3.11/site-packages/astor/node_util.py", line 143, in visit
    return visitor(node)
           ^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/astor/code_gen.py", line 876, in visit_Module
    self.write(*node.body)
  File "/usr/local/lib/python3.11/site-packages/astor/code_gen.py", line 177, in write
    visit(item)
  File "/usr/local/lib/python3.11/site-packages/astor/node_util.py", line 143, in visit
    return visitor(node)
           ^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/astor/code_gen.py", line 334, in visit_FunctionDef
    self.body(node.body)
  File "/usr/local/lib/python3.11/site-packages/astor/code_gen.py", line 225, in body
    self.write(*statements)
  File "/usr/local/lib/python3.11/site-packages/astor/code_gen.py", line 177, in write
    visit(item)
  File "/usr/local/lib/python3.11/site-packages/astor/node_util.py", line 143, in visit
    return visitor(node)
           ^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/astor/node_util.py", line 137, in abort_visit
    raise AttributeError(msg % node.__class__.__name__)
AttributeError: No defined handler for node of type Match
@BethanyG
Copy link
Member

BethanyG commented Mar 1, 2024

The library that was used to turn an AST back into code in the representer (astor) does not appear to be maintained for more recent versions of Python, so it chokes on things like structural pattern matching and the walrus operator (:=).

Fortunately core Python added ast.unparse in Python 3.8, and it was always slightly preferable to use only core Python for the representer anyways. Pull 45 replaces astor with ast.unparse, among other refactorings that fixes this bug.

edited to add: Actually, it does look like the astor folx have tried to bring the library up to date for match statements. However, I don't want to rely on them and have this break every time a new feature is added but the team is behind. I'd rather rely on the core Python team and the built-in AST lib since it is more stable and up-to-date.

Sadly, I've gotten sideswiped by writing a bunch of approaches and reviewing concepts that Colin is writing (and other life things) -- so I never got back to changing the representer to handle multiple student files, and we never merged Pull 45.

While I would love to do that final bit work , it feels as if maybe we merge what we have to fix this and other issues and do the representer re-runs?

I am happy to do the multi-file support, but if we wait on me for that, this bug (at the rate I am currently moving) is likely to go unfixed for another month or more.

@BethanyG
Copy link
Member

BethanyG commented Mar 1, 2024

One note: There aren't specific parsing tests for anything introduced in Python 3.9, Python 3.10, or Python 3.11, but I can add some for features I think might get high use from students. At this time, type annotations are being stripped, so there aren't any test cases needed there.

IIRC, I did make one case with pattern matching and one with a walrus. In any case, I will review and work on adding some of those today and over the weekend, just to cover our bases.

@ErikSchierboom
Copy link
Member Author

While I would love to do that final bit work , it feels as if maybe we merge what we have to fix this and other issues and do the representer re-runs?

I am happy to do the multi-file support, but if we wait on me for that, this bug (at the rate I am currently moving) is likely to go unfixed for another month or more.

As multi-file submissions will be the outlier (I can detect those and selectively re-run them), I'm totally happy with this proposed plan.

@BethanyG
Copy link
Member

BethanyG commented Mar 5, 2024

I added an issue for the multi-file handling, so I don't forget. 😄
I didn't get a chance to add more smoke tests. However, we do have one already for pattern matching, which is the big one here.

I'll make an issue to add more tests for newer features, but since the representer is now using ast.unparse, I think we're largely "safe".

@BethanyG
Copy link
Member

Linking PR #55 here, because in the course of writing more smoke tests, @brocla found a bug with unassigned expressions that needed fixing. This is why we test.... 😄

Since that PR is unmerged, I think I will keep this open for a little bit longer.

@BethanyG
Copy link
Member

Closing this as fixed, at least until we find another bug. 😄

@brocla
Copy link
Contributor

brocla commented Mar 28, 2024 via email

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

No branches or pull requests

3 participants