python-tutorial icon indicating copy to clipboard operation
python-tutorial copied to clipboard

Issues with "Exercise 4: Base converter" in Control Flow notebook

Open j-piller opened this issue 8 months ago • 1 comments

Describe the bugs A) Final string should be reverted, with e.g. result = result[::-1], to yield the correct result.

https://github.com/empa-scientific-it/python-tutorial/blob/cbd5572b1aff6875f5f75689217125cfcb28540a/tutorial/tests/test_02_control_flow.py#L412-L419

        decimal //= to_base

    #TODO: result = result[::-1] to revert the string
    # Test: e.g. -"A00", from_base=16, to_base=13 (2560 in base_10) should return -121C and not -C121)

    return f"-{result}" if is_negative else result

B) In the case of to_base=10 the sign is not added.

https://github.com/empa-scientific-it/python-tutorial/blob/cbd5572b1aff6875f5f75689217125cfcb28540a/tutorial/tests/test_02_control_flow.py#L409-L410 if to_base == 10: return str(decimal) #TODO: Handle negative numbers e.g. f'-{str(decimal)}' if is_negative else str(decimal)

To Reproduce Steps to reproduce the behavior: A. reference_base_converter("-A00", 16, 13) B. reference_base_converter("-A00", 16, 10) or any negative number converted from base!=10 to base=10

Expected behavior A: reference_base_converter("-A00", 16, 13) returns -121C [2560 in base 10] B: reference_base_converter("-A00", 16, 10) returns -2560

Where did you run the tutorial?

  • Local installation (conda: jupyter lab)

Additional context First GitHub reported issue, I apologize for anything unclear.

j-piller avatar May 13 '25 08:05 j-piller

Thanks @j-piller! Good catch of both these bugs.

There's an additional improvement to be made here: checking whether from_base == to_base is done before digits validation, so calling the reference solution like reference_base_converter('G', 10, 10) immediately returns "G", which is clearly wrong (and meaningless) in base 10. We should move digits validation before this check.

https://github.com/empa-scientific-it/python-tutorial/blob/cbd5572b1aff6875f5f75689217125cfcb28540a/tutorial/tests/test_02_control_flow.py#L382-L399

edoardob90 avatar May 13 '25 16:05 edoardob90