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

[BUG] Unclear message "list index out of range" #3463

Closed
qnojansen opened this issue Oct 19, 2022 · 7 comments · Fixed by #3788
Closed

[BUG] Unclear message "list index out of range" #3463

qnojansen opened this issue Oct 19, 2022 · 7 comments · Fixed by #3788
Assignees

Comments

@qnojansen
Copy link

Describe the bug
When choosing an item from an empty list, you get the error message "list index out of range".

Paste the Hedy code & level
Level 16
fruit = []
f = fruit at random
print f

Add a screenshot (optional)
See attached file

Expected behavior
The message should be more descriptive (and in Dutch) so students will understand it.

image

@LeslieMurphy
Copy link

LeslieMurphy commented Oct 25, 2022

Also a student can get error message list index out of range in level 3.

I propose to enhance Hedy to generate error message something like this "List animals contains 3 entries. Index 4 is out of range". What does everyone think?

Searching the source code, I think this error message is not generated from within the hedy interpreter! I think its a Python runtime exception error message, hence the lack of localization.

Weirdly print animals at 0 does the same thing as print animals at 3. Both print kangaroo. Is that another bug?

animals is dog, cat, kangaroo
print animals at 4
# The above code generates error: list index out of range

@Felienne
Copy link
Member

Ah great catch, thanks!! It is indeed a Python error message leaking out. We aim to not have that happen but it sometimes does. Here also it is quite hard to fix cause this happens at runtime.

Best we can do I think is to generate a try-catch and have that output an error (we have done that in other places)

@jpelay do you think you can pick this one up? If not I will do it!

@LeslieMurphy
Copy link

I can try to look into this to see if I can figure it out. Is there a closed issue with similar python runtime message leak I might review?

@Felienne
Copy link
Member

Felienne commented Oct 27, 2022 via email

@LeslieMurphy
Copy link

While I delve into this, I pose another related question if you do not mind.

Are lists in hedy supposed to be mutable by index? It does not seem to work (level 16 and above). Was trying to simplify the tic tac toe example into using a single dimension list and could not accomplish it. Current logic has 9 variables (spot_1, spot_2, spot_3 etc.).

friends = ['Ahmed', 'Ben', 'Cayden']
print friends[1]  # this prints Ahmed
friends = ['Fred', 'George', 'Fritz'] #reassignment
print friends[1]  # this prints Fred
friends[1]='Les'  # not valid Hedy code. There is a mistake on line 5, at position 8. You typed [, but that is not allowed.
lucky_numbers = [15, 18, 6]
for i in range 1 to 3
    print 'the lucky number of ' friends[i]
    print 'is ' lucky_numbers[i]


@LeslieMurphy
Copy link

LeslieMurphy commented Oct 28, 2022

For @qnojansen 's level 16 bug - unclear message when trying to get a random selection out of an empty list, this kludge in the code would work. It is not very user friendly, but it is better than the default python message. Ideally, the error would include the variable name of the list that is being accessed incorrectly.

in hedy.py

class ConvertToPython_5(ConvertToPython_4):
    def list_access_var(self, meta, args):
        var = escape_var(args[0])
        if isinstance(args[2], Tree):
            # protect in case of empty list
            return f"try:\n  {var} = random.choice({args[1]})\nexcept IndexError:\n  raise Exception('The list is empty, there is no random value!')"
            #return var + ' = random.choice(' + args[1] + ')'
        else:
            return var + ' = ' + args[1] + '[' + args[2] + '-1]'

image

Also, in level 16 an error can occur on an out of bounds list access. This is a fix for that:

def print(self, meta, args):
        argument_string = self.print_ask_args(meta, args)
        return f"try:\n  print(f'{argument_string}')\nexcept IndexError:\n  raise Exception('Sorry I cannot find that item in the list!')"

image

Finally for my level 3 bug, here is a fix for that. I am sure there are other cases lurking around as well. Probably need to review all unsafe access areas and wrap the code with try/catch more systematically.

def print(self, meta, args):
        args_new = []
        for a in args:
            # list access has been already rewritten since it occurs lower in the tree
            # so when we encounter it as a child of print it will not be a subtree, but
            # transpiled code (for example: random.choice(dieren))
            # therefore we should not process it anymore and thread it as a variable:
            # we set the line number to 100 so there is never an issue with variable access before
            # assignment (regular code will not work since random.choice(dieren) is never defined as var as such)
            if "random.choice" in a or "[" in a:
                args_new.append(self.process_variable_for_fstring(a, meta.line))
                argument_string = ' '.join(args_new)
                return f"try:\n  print(f'{argument_string}')\nexcept IndexError:\n  raise Exception('Sorry I cannot find that item in the list!')"
            else:
                ...

image

@LeslieMurphy
Copy link

I found another Python message leak.

cannot concatenate 'str' and 'int' objects

Shouldn't it be possible to concatenate a string and text in Hedy? I was looking at simplifying my multiplication table formatting logic, and this approach currently won't work:

image

@mergify mergify bot closed this as completed in #3788 Jan 2, 2023
mergify bot pushed a commit that referenced this issue Jan 2, 2023
**Description**

Adds an exception to the code generated after transpiling to catch index out of bounds errors.
Things to do:
- [x] Figure out a way to make the message translatable (`gettext` works inside hedy.py!)
- [x] Try to show which line and variable caused the error. (Can show the variable, not the line!)

**Fixes #3463**

**How to test**

Since this is added **after** transpiling, should I add an e2e test?

**Checklist**
Done? Check if you have it all in place using this list:*
  
- [ ] Contains one of the PR categories in the name
- [ ] Describes changes in the format above
- [ ] Links to an existing issue or discussion 
- [ ] Has a "How to test" section

If you're unsure about any of these, don't hesitate to ask. We're here to help!
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.

4 participants