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

Warn user about tuple flattening in precedence table #278

Closed
mrontio opened this issue Dec 11, 2022 · 3 comments
Closed

Warn user about tuple flattening in precedence table #278

mrontio opened this issue Dec 11, 2022 · 3 comments

Comments

@mrontio
Copy link

mrontio commented Dec 11, 2022

When creating a precedence table, the example in the documentation suggests to do something like this:

precedence = (
     ('left', 'PLUS', 'MINUS'),
     ('left', 'TIMES', 'DIVIDE'),
 )

There's a very easy mistake to make here, if you have a single entry in the table and you miss the end comma, e.g.

precedence = (
    ('left', 'ARROW')
)

Python flattens the tuple, so the precedence would simply be ('left', 'ARROW') instead of (('left', 'ARROW'),).
The way to fix this is to simply add a , to the end of the entry, but its not clear what the issue is once you make a mistake.
This flattening trips the following line:

if not isinstance(p, (list, tuple)):

My suggestion is to make this line a bit more descriptive, including what it expected, what it got, and a note about tuple flattening:

self.log.error('Bad precedence table')

I intend on making a pull request with a solution later.

@mrontio mrontio closed this as completed Dec 11, 2022
@mrontio
Copy link
Author

mrontio commented Dec 11, 2022

My bad. Didn't read the README.

@dabeaz
Copy link
Owner

dabeaz commented Dec 11, 2022

I would accept a pull request for this.

@mrontio
Copy link
Author

mrontio commented Dec 12, 2022

Fantastic, I do think it will save some time in the labs.

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

Successfully merging a pull request may close this issue.

2 participants