Skip to content

Commit

Permalink
[FIX] - Comparing int and float gives wrong answer (hedyorg#3948)
Browse files Browse the repository at this point in the history
## Description

* `convert_numerals` function now returns an int, float or string accordingly
* no longer necessary to pad with leading zeros 

Fixes hedyorg#3726

## How to test

### Automated Unit test:

```
python -m pytest tests/test_level/test_level_14.py -k 'test_greater_than_with_int_and_float or test_not_greater_than_with_int_and_float'
```

### Manual UI test:

1. reproduce bad behavior
2. apply fix
3. confirm corrected behavior

#### To reproduce the broken behavior, before this fix is applied:

Navigate to Level 14, enter and run the following code:

```
age = 16
if age > 15.0
    print 'You are older than me!'
else
    print 'You are younger than me!'

age = 7.0
if age > 15
    print 'You are older than me!'
else
    print 'You are younger than me!'
```

expected output

```
You are older than me!
You are younger than me!
```

actual output ☹

```
You are younger than me!
You are older than me!
```

#### To confirm fix after it has been applied:

Execute the reproduction steps above, and confirm that now the expected output matches the actual output 😃
  • Loading branch information
noelgolding authored Jan 20, 2023
1 parent dd147b2 commit e68ac9b
Show file tree
Hide file tree
Showing 11 changed files with 4,190 additions and 4,110 deletions.
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ coverage.xml
*.py,cover
.hypothesis/
.pytest_cache/
tests/cypress/screenshots/

# Translations -> the .mo files are automatically compiled on deploy
*.mo
Expand Down
19 changes: 6 additions & 13 deletions hedy.py
Original file line number Diff line number Diff line change
Expand Up @@ -1276,14 +1276,14 @@ def process_variable_for_fstring(self, variable_name, access_line_number=100):
else:
return process_characters_needing_escape(variable_name)

def process_variable_for_fstring_padded(self, name):
def process_variable_for_comparisons(self, name):
# used to transform variables in comparisons
if self.is_variable(name):
return f"convert_numerals('{self.numerals_language}', {escape_var(name)}).zfill(100)"
return f"convert_numerals('{self.numerals_language}', {escape_var(name)})"
elif ConvertToPython.is_float(name):
return f"convert_numerals('{self.numerals_language}', {name}).zfill(100)"
return f"convert_numerals('{self.numerals_language}', {name})"
elif ConvertToPython.is_quoted(name):
return f"{name}.zfill(100)"
return f"{name}"

def make_f_string(self, args):
argument_string = ''
Expand Down Expand Up @@ -2217,15 +2217,8 @@ def or_condition(self, meta, args):
class ConvertToPython_14(ConvertToPython_13):
def process_comparison(self, meta, args, operator):

# we are generating an fstring now
arg0 = self.process_variable_for_fstring_padded(args[0])
arg1 = self.process_variable_for_fstring_padded(args[1])

# zfill(100) in process_variable_for_fstring_padded leftpads variables to length 100 with zeroes (hence the z fill)
# that is to make sure that string comparison works well "ish" for numbers
# this at one point could be improved with a better type system, of course!
# the issue is that we can't do everything in here because
# kids submit things with the ask command that wew do not ask them to cast (yet)
arg0 = self.process_variable_for_comparisons(args[0])
arg1 = self.process_variable_for_comparisons(args[1])

simple_comparison = arg0 + operator + arg1

Expand Down
Loading

0 comments on commit e68ac9b

Please sign in to comment.