Trouble parsing unassigned expressions

[brocla]

The Representer fails to parse this dataclass,

from dataclasses import dataclass


@dataclass
class Point:
    x: int
    y: int

Here is the traceback.

But it will parse the dataclass if the variables are initialized like this,

from dataclasses import dataclass


@dataclass
class Point:
    x: int = 0
    y: int = 0

The representer also fails on this bit of code,

try:
    1/0
except:
    pass

but will parse the code if the expression is assigned like this,

try:
    x = 1/0
except:
    pass

Even though the traceback points at ast.unparse(tree), I suspect the visit_Expr() method is the place to look.

[BethanyG]

sigh. That auto-close bot sure is aggressive.

Even though the traceback points at ast.unparse(tree), I suspect the visit_Expr() method is the place to look.

Yup. We may have an issue there. But basic math doesn't fail, AFAK, so maybe it's the nesting.

Parsing and unparsing the code directly using ast.parse() and ast.unparse works just fine. But it is weird that an uninstantiated dataclass (which is a shortcut class) would create any issues at all. Especially since uninstantiated classes don't create issues.

🤔 .....although, it might be the decorator specifically. That doesn't explain the issues with the try -- except.

I'll take a look this afternoon. Thanks for reporting this!

[BethanyG]

UPDATE:

Starting at line 314

This code:

        # Pass through generator code.
        if isinstance(node.value, Yield):
            return node

Did not account for other possible top-level unassigned expressions. For the most part, this wouldn't get triggered by student code because most things inside functions get assigned names (...right up until they don't get assigned names 🤦🏽‍♀️ ). But a biggie is that it didn't account for hanging lambda expressions. UGH.

So that we have a record, the ast.Expr types we needed to account for in the above are:

UnaryOp, BinOp, BoolOp, Lambda, ListComp, DictComp, SetComp, GenExpr, Yield, YieldFrom

However, if we look explicitly for Constant, we don't have to be explicit with the skips.
So, I eliminated that code, and adjusted the eliminate registered docstrings code to read:

        if isinstance(node.value, Constant) and not isinstance(node.value, Call):
            # eliminate registered docstrings
            if utils.md5sum(node.value.value) in self._docstring_cache:
                return None

But I now have some tests failing, so need to make sure I haven't munged something else. Will keep you posted. 😄

[BethanyG]

Alrighty. PR'd a fix for this here. Should you want to play with it using docker, you will need to pull the PR branch and rebuild the container by using docker build -t exercism/python-representer . from your exercism/python-representer directory.

Let me know if you have questions or issues. Many thanks again for finding this. 😄

We may need to wait until Erik is back from vacation to merge this. I don't think that's a huge deal, but if you think this migtht be critical, we can ask iHiD to review and merge for us.

[brocla]

Thanks for fixing this! 😊 I'll go try it out. I think it is important to do, but I don't think it is time critical.

…

On Thu, Mar 21, 2024, 5:39 PM BethanyG ***@***.***> wrote: Alrighty. PR'd a fix for this here <#55>. Should you want to play with it using docker, you will need to pull the PR branch and rebuild the container by using docker build -t exercism/python-representer . from your exercism/python-representer directory. Let me know if you have questions or issues. Many thanks again for finding this. 😄 We may need to wait until Erik is back from vacation to merge this. I don't think that's a huge deal, but if you think this migth be critical, we can ask iHiD to review and merge for us. — Reply to this email directly, view it on GitHub <#54 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACBULKUOPAH4B4GSSJJA2NLYZNVUBAVCNFSM6AAAAABFBV3O4SVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAMJUGA2TCNRXGI> . You are receiving this because you authored the thread.Message ID: ***@***.***>

[BethanyG]

PR #55 has been merged 🎉 . Closing this as fixed (at least until we find the next bug). 😄